fix(rclone): Use semver for version checking

Cargo.toml

@@ -18,6 +18,7 @@ rustic_core = { path = "crates/core" }
 simplelog = "0.12.2"
 
 # dev-dependencies
+rstest = "0.18.2"
 tempfile = "3.10.1"
 
 # see: https://nnethercote.github.io/perf-book/build-configuration.html

crates/backend/Cargo.toml

@@ -38,7 +38,7 @@ clap = ["dep:clap"]
 s3 = ["opendal"]
 opendal = ["dep:opendal", "dep:rayon", "dep:tokio", "tokio/rt-multi-thread"]
 rest = ["dep:reqwest", "dep:backoff"]
-rclone = ["rest", "dep:rand"]
+rclone = ["rest", "dep:rand", "dep:semver"]
 # Note: sftp is not yet supported on windows, see below
 sftp = ["opendal"]
 
@@ -82,6 +82,7 @@ reqwest = { version = "0.11.25", default-features = false, features = ["json", "
 
 # rclone backend
 rand = { version = "0.8.5", optional = true }
+semver = { version = "1.0.22", optional = true }
 
 # opendal backend
 rayon = { version = "1.9.0", optional = true }
@@ -96,7 +97,7 @@ opendal = { version = "0.45", features = ["services-b2", "services-sftp", "servi
 opendal = { version = "0.45", features = ["services-b2", "services-swift", "layers-blocking"], optional = true }
 
 [dev-dependencies]
-rstest = "0.18.2"
+rstest = { workspace = true }
 
 [lints]
 workspace = true

crates/backend/src/error.rs

@@ -54,6 +54,8 @@ pub enum RcloneErrorKind {
     FromUtf8Error(#[from] Utf8Error),
     /// error parsing verision number from `{0:?}`
     FromParseVersion(String),
+    /// Using rclone without authentication! Upgrade to rclone >= 1.52.2 (current version: `{0}`)!
+    RCloneWithoutAuthentication(String),
 }
 
 /// [`RestErrorKind`] describes the errors that can be returned while dealing with the REST API

crates/backend/src/rclone.rs

@@ -7,12 +7,13 @@ use std::{
 
 use anyhow::Result;
 use bytes::Bytes;
-use itertools::Itertools;
-use log::{debug, info, warn};
+use log::{debug, info};
 use rand::{
     distributions::{Alphanumeric, DistString},
     thread_rng,
 };
+
+use semver::{BuildMetadata, Prerelease, Version, VersionReq};
 use shell_words::split;
 
 use crate::{error::RcloneErrorKind, rest::RestBackend};
@@ -47,7 +48,11 @@ impl Drop for RcloneBackend {
     }
 }
 
-/// Get the rclone version.
+/// Check the rclone version.
+///
+/// # Arguments
+///
+/// * `rclone_version_output` - The output of `rclone version`.
 ///
 /// # Errors
 ///
@@ -58,31 +63,40 @@ impl Drop for RcloneBackend {
 ///
 /// # Returns
 ///
-/// The rclone version as a tuple of (major, minor, patch).
+/// * `Ok(())` - If the rclone version is supported.
 ///
 /// [`RcloneErrorKind::FromIoError`]: RcloneErrorKind::FromIoError
 /// [`RcloneErrorKind::FromUtf8Error`]: RcloneErrorKind::FromUtf8Error
 /// [`RcloneErrorKind::NoOutputForRcloneVersion`]: RcloneErrorKind::NoOutputForRcloneVersion
 /// [`RcloneErrorKind::FromParseVersion`]: RcloneErrorKind::FromParseVersion
-fn rclone_version() -> Result<(i32, i32, i32)> {
-    let rclone_version_output = Command::new("rclone")
-        .arg("version")
-        .output()
-        .map_err(RcloneErrorKind::FromIoError)?
-        .stdout;
-    let rclone_version = std::str::from_utf8(&rclone_version_output)
+fn check_clone_version(rclone_version_output: &[u8]) -> Result<()> {
+    let rclone_version = std::str::from_utf8(rclone_version_output)
         .map_err(RcloneErrorKind::FromUtf8Error)?
         .lines()
         .next()
         .ok_or_else(|| RcloneErrorKind::NoOutputForRcloneVersion)?
         .trim_start_matches(|c: char| !c.is_numeric());
 
-    let versions = rclone_version
-        .split(&['.', '-', ' '][..])
-        .filter_map(|v| v.parse().ok())
-        .collect_tuple()
-        .ok_or_else(|| RcloneErrorKind::FromParseVersion(rclone_version.to_string()))?;
-    Ok(versions)
+    let mut parsed_version = Version::parse(rclone_version)?;
+
+    // we need to set the pre and build fields to empty to make the comparison work
+    // otherwise the comparison will take the pre and build fields into account
+    // which would make beta versions pass the check
+    parsed_version.pre = Prerelease::EMPTY;
+    parsed_version.build = BuildMetadata::EMPTY;
+
+    // for rclone < 1.52.2 setting user/password via env variable doesn't work. This means
+    // we are setting up an rclone without authentication which is a security issue!
+    // we hard fail here to prevent this, as we can't guarantee the security of the data
+    // also because 1.52.2 has been released on Jun 24, 2020, we can assume that this is a
+    // reasonable lower bound for the version
+    if VersionReq::parse("<1.52.2")?.matches(&parsed_version) {
+        return Err(
+            RcloneErrorKind::RCloneWithoutAuthentication(rclone_version.to_string()).into(),
+        );
+    }
+
+    Ok(())
 }
 
 impl RcloneBackend {
@@ -121,21 +135,16 @@ impl RcloneBackend {
             .unwrap_or(true);
 
         if use_password && rclone_command.is_none() {
-            // if we want to use a password and rclone_command is not explicitely set, we check for a rclone version supporting
-            // user/password via env variables
-            match rclone_version() {
-                Ok(v) => {
-                    if v < (1, 52, 2) {
-                        // TODO: This should be an error, and explicitly agreed to with a flag passed to `rustic`,
-                        // check #812 for details
-                        // for rclone < 1.52.2 setting user/password via env variable doesn't work. This means
-                        // we are setting up an rclone without authentication which is a security issue!
-                        // (however, it still works, so we give a warning)
-                        warn!("Using rclone without authentication! Upgrade to rclone >= 1.52.2 (current version: {}.{}.{})!", v.0, v.1, v.2);
-                    }
-                }
-                Err(err) => warn!("Could not determine rclone version: {err}"),
-            }
+            let rclone_version_output = Command::new("rclone")
+                .arg("version")
+                .output()
+                .map_err(RcloneErrorKind::FromIoError)?
+                .stdout;
+
+            // if we want to use a password and rclone_command is not explicitly set,
+            // we check for a rclone version supporting user/password via env variables
+            // if the version is not supported, we return an error
+            check_clone_version(rclone_version_output.as_slice())?;
         }
 
         let user = Alphanumeric.sample_string(&mut thread_rng(), 12);
@@ -308,3 +317,26 @@ impl WriteBackend for RcloneBackend {
         self.rest.remove(tpe, id, cacheable)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    use rstest::rstest;
+
+    #[rstest]
+    #[case(b"rclone v1.52.2\n- os/arch: linux/amd64\n- go version: go1.14.4\n")]
+    #[case(b"rclone v1.66.0\n- os/version: Microsoft Windows 11 Pro 23H2 (64 bit)\n- os/kernel: 10.0.22631.3155 (x86_64)\n- os/type: windows\n- os/arch: amd64\n- go/version: go1.22.1\n- go/linking: static\n- go/tags: cmount")]
+    #[case(b"rclone v1.63.0-beta.7022.e649cf4d5\n- os/arch: linux/amd64\n- go version: go1.14.4\n")]
+    fn test_check_clone_version_passes(#[case] rclone_version_output: &[u8]) {
+        assert!(check_clone_version(rclone_version_output).is_ok());
+    }
+
+    #[rstest]
+    #[case(b"")]
+    #[case(b"rclone v1.52.1\n- os/arch: linux/amd64\n- go version: go1.14.4\n")]
+    #[case(b"rclone v1.51.3-beta\n- os/arch: linux/amd64\n- go version: go1.14.4\n")]
+    fn test_check_clone_version_fails(#[case] rclone_version_output: &[u8]) {
+        assert!(check_clone_version(rclone_version_output).is_err());
+    }
+}

crates/core/Cargo.toml

@@ -136,7 +136,7 @@ mockall = "0.12.1"
 pretty_assertions = "1.4.0"
 quickcheck = "1.0.3"
 quickcheck_macros = "1.0.0"
-rstest = "0.18.2"
+rstest = { workspace = true }
 rustdoc-json = "0.8.9"
 # We need to have rustic_backend here, because the doc-tests in lib.rs of rustic_core
 rustic_backend = { workspace = true }