Repository: nushell/nushell
File: crates/nu-command/src/filesystem/ls.rs (Rust)

Code before:
            // https://github.com/golang/go/blob/016d7552138077741a9c3fdadc73c0179f5d3ff7/src/os/types_windows.go#L104-L105
            if find_data.dwReserved0 == IO_REPARSE_TAG_SYMLINK
                || find_data.dwReserved0 == IO_REPARSE_TAG_MOUNT_POINT
            {
                return true;
            }
        }
        false
    }
}

#[allow(clippy::type_complexity)]
fn read_dir(
    f: &Path,
    span: Span,
) -> Result<Box<dyn Iterator<Item = Result<PathBuf, ShellError>> + Send>, ShellError> {
    let iter = f
        .read_dir()
        .map_err(|error| {
            if error.kind() == std::io::ErrorKind::PermissionDenied {
                #[cfg(unix)]
                let error_msg = format!(
                    "The permissions of {:o} do not allow access for this user",
                    f.metadata()
                        .map(|m| m.permissions().mode() & 0o0777)
                        .expect("this shouldn't be called since we already know there is a dir")
                );
                #[cfg(not(unix))]
                let error_msg = String::from("Permission denied");
                return ShellError::GenericError {
                    error: "Permission denied".into(),
                    msg: error_msg,
                    span: Some(span),
                    help: None,
                    inner: vec![],
                };
            }

            error.into()
        })?
        .map(|d| d.map(|r| r.path()).map_err(|e| e.into()));
    Ok(Box::new(iter))
}


Review comment (security):
I think there may be a minor TOCTOU risk of this panicking if the directory is deleted between the raising of the error and hitting `File::metadata` which runs a `statx()` syscall on the file descriptor. 

Not quite sure how I feel about showing the octal mode repr here. Certainly more informative than bare permission denied but at the same time somewhat cryptic for folks not brought up with old `chmod`. And also not sure if it can properly represent dernied permissions due to ACLs.


Code after:
    fn is_symlink(find_data: &WIN32_FIND_DATAW) -> bool {
        if find_data.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT.0 != 0 {
            // Follow Golang's lead in treating mount points as symlinks.
            // https://github.com/golang/go/blob/016d7552138077741a9c3fdadc73c0179f5d3ff7/src/os/types_windows.go#L104-L105
            if find_data.dwReserved0 == IO_REPARSE_TAG_SYMLINK
                || find_data.dwReserved0 == IO_REPARSE_TAG_MOUNT_POINT
            {
                return true;
            }
        }
        false
    }
}

#[allow(clippy::type_complexity)]
fn read_dir(
    f: &Path,
    span: Span,
    use_threads: bool,
) -> Result<Box<dyn Iterator<Item = Result<PathBuf, ShellError>> + Send>, ShellError> {
    let items = f
        .read_dir()
        .map_err(|error| {
            if error.kind() == std::io::ErrorKind::PermissionDenied {
                return ShellError::GenericError {
                    error: "Permission denied".into(),
                    msg: "The permissions may not allow access for this user".into(),
                    span: Some(span),
                    help: None,
                    inner: vec![],
                };
            }

            error.into()
        })?
        .map(|d| {
            d.map(|r| r.path())
                .map_err(|e| ShellError::IOError { msg: e.to_string() })
        });
    if !use_threads {
        let mut collected = items.collect::<Vec<_>>();
        collected.sort_by(|a, b| {
            let a = a.as_ref().expect("path should be valid");
            let b = b.as_ref().expect("path should be valid");
            a.cmp(b)
        });
        return Ok(Box::new(collected.into_iter()));
    }
    Ok(Box::new(items))
}