From 3ff360a97185369c6992e83883747cbfb5a9ccf7 Mon Sep 17 00:00:00 2001 From: Charles GTE Date: Wed, 9 Sep 2026 20:17:03 +0200 Subject: [PATCH 1/9] build: install rclone 1.75.1 in agent images --- docker/Dockerfile | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/docker/Dockerfile b/docker/Dockerfile index 221c393..b0e6243 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -44,6 +44,15 @@ RUN ARCH=$(uname -m | sed 's/x86_64/amd64/;s/aarch64/arm64/') \ | tar -xjf - -C /usr/local/bin sqlcmd \ && chmod +x /usr/local/bin/sqlcmd +# ========================= +# rclone (all storage backends) +# ========================= +ARG RCLONE_VERSION=1.75.1 +RUN ARCH=$(dpkg --print-architecture) \ + && curl -fsSL -o /tmp/rclone.deb "https://downloads.rclone.org/v${RCLONE_VERSION}/rclone-v${RCLONE_VERSION}-linux-${ARCH}.deb" \ + && dpkg -i /tmp/rclone.deb \ + && rm /tmp/rclone.deb + ARG TARGETARCH # ========================= @@ -134,6 +143,12 @@ RUN apt-get update && apt-get install -y \ firebird3.0-utils \ && rm -rf /var/lib/apt/lists/* +ARG RCLONE_VERSION=1.75.1 +RUN ARCH=$(dpkg --print-architecture) \ + && curl -fsSL -o /tmp/rclone.deb "https://downloads.rclone.org/v${RCLONE_VERSION}/rclone-v${RCLONE_VERSION}-linux-${ARCH}.deb" \ + && dpkg -i /tmp/rclone.deb \ + && rm /tmp/rclone.deb + ENV DOTNET_ROOT=/usr/local/dotnet RUN curl -sSL https://dot.net/v1/dotnet-install.sh -o /tmp/dotnet-install.sh \ && chmod +x /tmp/dotnet-install.sh \ From 59312215e98c9da6731cf4867fe95e2b7c626301 Mon Sep 17 00:00:00 2001 From: Charles GTE Date: Wed, 9 Sep 2026 20:24:16 +0200 Subject: [PATCH 2/9] build: exclude target/ and local artifacts from the docker context --- .dockerignore | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.dockerignore b/.dockerignore index 84903c0..1653020 100644 --- a/.dockerignore +++ b/.dockerignore @@ -9,3 +9,10 @@ RELEASE.md #IDE configurations .idea + +# Rust build artifacts (31GB; the builder stage compiles from source) +target + +# Local runtime and tooling artifacts +dump.rdb +.superpowers From 9409f0ff25964565674b275b7bdf4d8338e03548 Mon Sep 17 00:00:00 2001 From: Charles GTE Date: Wed, 9 Sep 2026 20:31:50 +0200 Subject: [PATCH 3/9] feat(storage): add rclone config model and validation helpers --- src/services/storage/providers/mod.rs | 1 + .../storage/providers/rclone/helpers.rs | 72 ++++++++++ src/services/storage/providers/rclone/mod.rs | 2 + .../storage/providers/rclone/models.rs | 15 ++ src/tests/storage/mod.rs | 1 + src/tests/storage/rclone.rs | 133 ++++++++++++++++++ 6 files changed, 224 insertions(+) create mode 100644 src/services/storage/providers/rclone/helpers.rs create mode 100644 src/services/storage/providers/rclone/mod.rs create mode 100644 src/services/storage/providers/rclone/models.rs create mode 100644 src/tests/storage/rclone.rs diff --git a/src/services/storage/providers/mod.rs b/src/services/storage/providers/mod.rs index 536f103..ddf6a82 100644 --- a/src/services/storage/providers/mod.rs +++ b/src/services/storage/providers/mod.rs @@ -2,4 +2,5 @@ pub mod azure_blob; pub mod google_cloud_storage; pub mod google_drive; pub mod local; +pub mod rclone; pub mod s3; diff --git a/src/services/storage/providers/rclone/helpers.rs b/src/services/storage/providers/rclone/helpers.rs new file mode 100644 index 0000000..3ff638e --- /dev/null +++ b/src/services/storage/providers/rclone/helpers.rs @@ -0,0 +1,72 @@ +use anyhow::{Result, bail}; + +/// Backends that would give a storage channel read/write access to the agent or +/// dashboard container filesystem. Rejected here as well as in the dashboard's +/// zod schema, because the agent receives this config over the wire. +const BLOCKED_BACKEND_TYPES: [&str; 2] = ["local", "alias"]; + +/// Section headers and their `type =` values, in file order. +fn sections(config_text: &str) -> Vec<(String, Option)> { + let mut out: Vec<(String, Option)> = Vec::new(); + + for line in config_text.lines() { + let line = line.trim(); + + if line.starts_with('[') && line.ends_with(']') && line.len() > 2 { + out.push((line[1..line.len() - 1].trim().to_string(), None)); + continue; + } + + let Some((key, value)) = line.split_once('=') else { + continue; + }; + + if key.trim().eq_ignore_ascii_case("type") + && let Some(current) = out.last_mut() + && current.1.is_none() + { + current.1 = Some(value.trim().to_ascii_lowercase()); + } + } + + out +} + +/// Rejects a config that names a missing remote or reaches a blocked backend. +/// Every section is checked, not only `remote_name` — a `crypt` remote can wrap +/// a `local` one, and checking only the named section would let that through. +pub fn validate_config(config_text: &str, remote_name: &str) -> Result<()> { + let sections = sections(config_text); + + if sections.is_empty() { + bail!("rclone config contains no remote sections"); + } + + for (name, backend) in §ions { + let Some(backend) = backend else { continue }; + if BLOCKED_BACKEND_TYPES.contains(&backend.as_str()) { + bail!("rclone backend type '{backend}' is not allowed (remote '{name}')"); + } + } + + if !sections.iter().any(|(name, _)| name == remote_name) { + let available: Vec<&str> = sections.iter().map(|(name, _)| name.as_str()).collect(); + bail!( + "remote '{remote_name}' is not defined in the rclone config (available: {})", + available.join(", ") + ); + } + + Ok(()) +} + +/// `:/`, collapsing an empty path. +pub fn remote_target(remote_name: &str, remote_path: &str, remote_file_path: &str) -> String { + let base = remote_path.trim().trim_matches('/'); + + if base.is_empty() { + format!("{remote_name}:{remote_file_path}") + } else { + format!("{remote_name}:{base}/{remote_file_path}") + } +} diff --git a/src/services/storage/providers/rclone/mod.rs b/src/services/storage/providers/rclone/mod.rs new file mode 100644 index 0000000..29dd673 --- /dev/null +++ b/src/services/storage/providers/rclone/mod.rs @@ -0,0 +1,2 @@ +pub mod helpers; +pub mod models; diff --git a/src/services/storage/providers/rclone/models.rs b/src/services/storage/providers/rclone/models.rs new file mode 100644 index 0000000..849bea6 --- /dev/null +++ b/src/services/storage/providers/rclone/models.rs @@ -0,0 +1,15 @@ +use serde::{Deserialize, Serialize}; + +/// Deserialized from `DatabaseStorage.config`. Keys arrive camelCase from the +/// dashboard and are converted by `deserialize_snake_case` before this struct +/// sees them, so no serde rename is needed — same as `S3ProviderConfig`. +#[derive(Debug, Deserialize, Serialize)] +pub struct RcloneProviderConfig { + /// The raw rclone config file the user pasted. May hold several sections. + pub config_text: String, + /// Which section of `config_text` is the upload target. + pub remote_name: String, + /// Optional prefix inside the remote, e.g. `my-bucket`. May be empty. + /// `backups//` is appended to it by `full_file_path`. + pub remote_path: String, +} diff --git a/src/tests/storage/mod.rs b/src/tests/storage/mod.rs index c67074e..71e00d1 100644 --- a/src/tests/storage/mod.rs +++ b/src/tests/storage/mod.rs @@ -1,2 +1,3 @@ mod azure_blob; mod google_cloud_storage; +mod rclone; diff --git a/src/tests/storage/rclone.rs b/src/tests/storage/rclone.rs new file mode 100644 index 0000000..1ab3ad3 --- /dev/null +++ b/src/tests/storage/rclone.rs @@ -0,0 +1,133 @@ +use crate::services::api::models::agent::status::DatabaseStorage; +use crate::services::storage::providers::rclone::helpers::{remote_target, validate_config}; +use crate::services::storage::providers::rclone::models::RcloneProviderConfig; +use crate::tests::init_tracing_for_test; +use crate::utils::file::full_file_path; + +const OVH_CONFIG: &str = "[ovhcloud-rbx]\n\ + type = s3\n\ + provider = OVHcloud\n\ + access_key_id = my_access\n\ + secret_access_key = my_secret\n\ + region = rbx\n\ + endpoint = s3.rbx.io.cloud.ovh.net\n\ + acl = private\n"; + +#[test] +fn config_deserializes_from_dashboard_camel_case() { + init_tracing_for_test(); + + // Exactly the shape the dashboard puts on the wire: camelCase keys inside + // `config`, converted to snake_case by `deserialize_snake_case`. + let storage: DatabaseStorage = serde_json::from_value(serde_json::json!({ + "id": "storage-1", + "provider": "rclone", + "folderName": "backups", + "config": { + "configText": OVH_CONFIG, + "remoteName": "ovhcloud-rbx", + "remotePath": "my-bucket", + } + })) + .unwrap(); + + let config: RcloneProviderConfig = storage.config.try_into().unwrap(); + + assert_eq!(config.remote_name, "ovhcloud-rbx"); + assert_eq!(config.remote_path, "my-bucket"); + assert!(config.config_text.contains("type = s3")); +} + +#[test] +fn validate_config_accepts_the_target_remote() { + assert!(validate_config(OVH_CONFIG, "ovhcloud-rbx").is_ok()); +} + +#[test] +fn validate_config_rejects_an_unknown_remote_name() { + let err = validate_config(OVH_CONFIG, "typo").unwrap_err().to_string(); + assert!(err.contains("typo"), "unexpected error: {err}"); + assert!(err.contains("ovhcloud-rbx"), "error should list the available remotes: {err}"); +} + +#[test] +fn validate_config_rejects_local_backend() { + let cfg = "[disk]\ntype = local\n"; + let err = validate_config(cfg, "disk").unwrap_err().to_string(); + assert!(err.contains("local"), "unexpected error: {err}"); +} + +#[test] +fn validate_config_rejects_alias_backend() { + let cfg = "[shortcut]\ntype = alias\nremote = other:path\n"; + let err = validate_config(cfg, "shortcut").unwrap_err().to_string(); + assert!(err.contains("alias"), "unexpected error: {err}"); +} + +#[test] +fn validate_config_rejects_a_blocked_backend_in_a_chained_section() { + // The target remote is fine, but it wraps a `local` remote. Checking only the + // named section would let this through. + let cfg = "[secret]\ntype = crypt\nremote = disk:vault\n\n[disk]\ntype = local\n"; + let err = validate_config(cfg, "secret").unwrap_err().to_string(); + assert!(err.contains("local"), "unexpected error: {err}"); + assert!(err.contains("disk"), "error should name the offending remote: {err}"); +} + +#[test] +fn validate_config_accepts_a_chained_crypt_over_s3() { + let cfg = format!("[secret]\ntype = crypt\nremote = ovhcloud-rbx:bucket\n\n{OVH_CONFIG}"); + assert!(validate_config(&cfg, "secret").is_ok()); +} + +#[test] +fn remote_path_is_a_prefix_ahead_of_the_backup_folder() { + // `remotePath` points at storage that may hold other things; every backup + // lands under its own `backups/` subtree beneath it. + assert_eq!( + remote_target("ovhcloud-rbx", "my-bucket", "backups/2026-09-09/x.tar.gz"), + "ovhcloud-rbx:my-bucket/backups/2026-09-09/x.tar.gz" + ); + + // Deeper prefixes nest the same way. + assert_eq!( + remote_target("ovhcloud-rbx", "my-bucket/portabase", "backups/2026-09-09/x.tar.gz"), + "ovhcloud-rbx:my-bucket/portabase/backups/2026-09-09/x.tar.gz" + ); +} + +#[test] +fn remote_target_trims_surrounding_slashes_and_whitespace() { + assert_eq!( + remote_target("r", " /my-bucket/ ", "a/b.bin"), + "r:my-bucket/a/b.bin" + ); +} + +#[test] +fn remote_target_handles_an_empty_remote_path() { + assert_eq!(remote_target("r", "", "a/b.bin"), "r:a/b.bin"); + assert_eq!(remote_target("r", " ", "a/b.bin"), "r:a/b.bin"); +} + +#[test] +fn an_empty_remote_path_falls_back_to_the_global_backup_folder() { + // remotePath is optional. When it is empty the destination comes entirely + // from `full_file_path`, which the dashboard drives with + // folderName = getBackupFolderName() (BACKUP_FOLDER_NAME, default "backups") + // and which defaults to "backups" again on its own if that is absent. + // No fallback code of our own — this test pins the composed result. + let remote_file_path = full_file_path(&"x.tar.gz".to_string(), None); + assert!(remote_file_path.starts_with("backups/")); + + assert_eq!( + remote_target("ovhcloud-rbx", "", &remote_file_path), + format!("ovhcloud-rbx:{remote_file_path}") + ); + + // Setting remotePath only prepends; the backups// tail is unchanged. + assert_eq!( + remote_target("ovhcloud-rbx", "my-bucket", &remote_file_path), + format!("ovhcloud-rbx:my-bucket/{remote_file_path}") + ); +} From b222b1393411c83f46e37d9fd3b6b53ee1692d51 Mon Sep 17 00:00:00 2001 From: Charles GTE Date: Wed, 9 Sep 2026 21:54:26 +0200 Subject: [PATCH 4/9] feat(storage): stream backups into rclone rcat --- Cargo.toml | 2 +- .../storage/providers/rclone/helpers.rs | 87 ++++++++++++- src/tests/storage/rclone.rs | 122 ++++++++++++++++++ 3 files changed, 209 insertions(+), 2 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 65d9cc2..bbd68ff 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,7 +19,7 @@ log = "0.4.29" toml = "0.9.10" reqwest = { version = "0.13.1", features = ["json", "blocking", "multipart", "stream", "query"] } anyhow = "1.0.100" -tokio = { version = "1.49.0", features = ["rt", "rt-multi-thread", "macros", "fs"] } +tokio = { version = "1.49.0", features = ["rt", "rt-multi-thread", "macros", "fs", "process", "io-util"] } async-trait = "0.1.89" tempfile = "3.24.0" openssl = "0.10.75" diff --git a/src/services/storage/providers/rclone/helpers.rs b/src/services/storage/providers/rclone/helpers.rs index 3ff638e..ae106e8 100644 --- a/src/services/storage/providers/rclone/helpers.rs +++ b/src/services/storage/providers/rclone/helpers.rs @@ -1,4 +1,14 @@ -use anyhow::{Result, bail}; +use anyhow::{Context, Result, bail}; +use bytes::Bytes; +use futures::{Stream, StreamExt}; +use std::io::Write; +use std::path::Path; +use std::pin::Pin; +use std::process::Stdio; +use tempfile::NamedTempFile; +use tokio::io::{AsyncReadExt, AsyncWriteExt}; +use tokio::process::Command; +use tracing::info; /// Backends that would give a storage channel read/write access to the agent or /// dashboard container filesystem. Rejected here as well as in the dashboard's @@ -70,3 +80,78 @@ pub fn remote_target(remote_name: &str, remote_path: &str, remote_file_path: &st format!("{remote_name}:{base}/{remote_file_path}") } } + +pub type RcloneStream = Pin> + Send>>; + +/// Writes the pasted config to an owner-only temp file. The file must stay +/// writable: rclone rewrites it in place when an OAuth backend refreshes its +/// access token. Deleted when the returned handle drops. +pub fn write_config(config_text: &str) -> Result { + let mut file = NamedTempFile::new().context("failed to create rclone config temp file")?; + + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(file.path(), std::fs::Permissions::from_mode(0o600)) + .context("failed to restrict rclone config permissions")?; + } + + file.write_all(config_text.as_bytes()) + .context("failed to write rclone config")?; + file.flush().context("failed to flush rclone config")?; + + Ok(file) +} + +/// Streams `stream` into `rclone rcat `. +/// +/// stderr is drained on its own task rather than via `wait_with_output`: rclone +/// can write to stderr while we are still feeding stdin, and a full stderr pipe +/// would block rclone forever while we block on the write. +pub async fn rcat(config_path: &Path, target: &str, mut stream: RcloneStream) -> Result<()> { + info!("rclone rcat -> {}", target); + + let mut child = Command::new("rclone") + .arg("--config") + .arg(config_path) + .arg("rcat") + .arg(target) + .stdin(Stdio::piped()) + .stdout(Stdio::null()) + .stderr(Stdio::piped()) + .spawn() + .context("failed to spawn rclone (is the binary installed in this image?)")?; + + let mut stderr_pipe = child.stderr.take().context("rclone stderr unavailable")?; + let stderr_task = tokio::spawn(async move { + let mut buf = String::new(); + let _ = stderr_pipe.read_to_string(&mut buf).await; + buf + }); + + let mut stdin = child.stdin.take().context("rclone stdin unavailable")?; + + while let Some(chunk) = stream.next().await { + // A stream error is ours, not rclone's — report it directly. + let chunk = chunk.context("backup stream failed")?; + + // A write error means rclone already exited. Stop pumping and let the + // exit status below produce the real reason; surfacing the broken-pipe + // error here would hide it. + if stdin.write_all(&chunk).await.is_err() { + break; + } + } + + let _ = stdin.flush().await; + drop(stdin); // EOF — rcat finalizes the upload only once stdin closes. + + let status = child.wait().await.context("failed to wait for rclone")?; + let stderr = stderr_task.await.unwrap_or_default(); + + if !status.success() { + bail!("rclone rcat failed ({status}): {}", stderr.trim()); + } + + Ok(()) +} diff --git a/src/tests/storage/rclone.rs b/src/tests/storage/rclone.rs index 1ab3ad3..1c28dd6 100644 --- a/src/tests/storage/rclone.rs +++ b/src/tests/storage/rclone.rs @@ -131,3 +131,125 @@ fn an_empty_remote_path_falls_back_to_the_global_backup_folder() { format!("ovhcloud-rbx:my-bucket/{remote_file_path}") ); } + +use crate::services::storage::providers::rclone::helpers::{rcat, write_config}; + +use bytes::Bytes; +use futures::stream; +use std::process::Command; +use testcontainers::core::{IntoContainerPort, WaitFor}; +use testcontainers::runners::AsyncRunner; +use testcontainers::{GenericImage, ImageExt}; + +const BUCKET: &str = "portabase"; + +async fn start_minio() -> (testcontainers::ContainerAsync, String) { + let container = GenericImage::new("minio/minio", "latest") + .with_exposed_port(9000.tcp()) + .with_wait_for(WaitFor::message_on_stderr("API:")) + .with_env_var("MINIO_ROOT_USER", "minioadmin") + .with_env_var("MINIO_ROOT_PASSWORD", "minioadmin") + .with_cmd(["server", "/data"]) + .start() + .await + .unwrap(); + + let host = container.get_host().await.unwrap().to_string(); + let port = container.get_host_port_ipv4(9000).await.unwrap(); + (container, format!("http://{host}:{port}")) +} + +fn minio_config(endpoint: &str) -> String { + format!( + "[minio]\n\ + type = s3\n\ + provider = Minio\n\ + access_key_id = minioadmin\n\ + secret_access_key = minioadmin\n\ + endpoint = {endpoint}\n\ + region = us-east-1\n\ + force_path_style = true\n" + ) +} + +/// Runs rclone synchronously and returns stdout, asserting a zero exit. +fn rclone_ok(config_path: &std::path::Path, args: &[&str]) -> Vec { + let out = Command::new("rclone") + .arg("--config") + .arg(config_path) + .args(args) + .output() + .expect("rclone binary not found — is it installed in this image?"); + + assert!( + out.status.success(), + "rclone {args:?} failed: {}", + String::from_utf8_lossy(&out.stderr) + ); + + out.stdout +} + +#[test] +fn write_config_creates_an_owner_only_file_with_the_exact_text() { + use std::os::unix::fs::PermissionsExt; + + let file = write_config(OVH_CONFIG).unwrap(); + + let mode = std::fs::metadata(file.path()).unwrap().permissions().mode(); + assert_eq!(mode & 0o777, 0o600, "config file must not be group/world readable"); + + assert_eq!(std::fs::read_to_string(file.path()).unwrap(), OVH_CONFIG); +} + +#[tokio::test] +async fn rcat_streams_a_multi_chunk_body_to_minio() { + init_tracing_for_test(); + + let (_container, endpoint) = start_minio().await; + let config = write_config(&minio_config(&endpoint)).unwrap(); + + rclone_ok(config.path(), &["mkdir", &format!("minio:{BUCKET}")]); + + // 10 KiB fed as 1 KiB chunks, so the stdin pump loops rather than doing one write. + let data = vec![7u8; 10 * 1024]; + let chunks: Vec> = data + .chunks(1024) + .map(|c| Ok(Bytes::copy_from_slice(c))) + .collect(); + + let target = remote_target("minio", BUCKET, "backups/2026-09-09/test.bin"); + + rcat(config.path(), &target, Box::pin(stream::iter(chunks))) + .await + .unwrap(); + + let got = rclone_ok(config.path(), &["cat", &target]); + assert_eq!(got, data); +} + +#[tokio::test] +async fn rcat_reports_rclone_stderr_when_the_remote_is_unreachable() { + init_tracing_for_test(); + + // Port 1 refuses connections, so rclone fails fast and closes stdin under us. + let config = write_config(&minio_config("http://127.0.0.1:1")).unwrap(); + + let chunks: Vec> = + vec![Ok(Bytes::from_static(&[0u8; 4096]))]; + + let err = rcat( + config.path(), + "minio:portabase/x.bin", + Box::pin(stream::iter(chunks)), + ) + .await + .expect_err("upload to an unreachable endpoint must fail"); + + let msg = err.to_string(); + assert!( + msg.contains("rclone rcat failed"), + "the broken stdin pipe must not mask rclone's own error: {msg}" + ); + assert!(!msg.trim().ends_with("failed"), "rclone stderr must be included: {msg}"); +} From ea356ed54fe937e5f5c2ea219cfd911728c3cdf7 Mon Sep 17 00:00:00 2001 From: Charles GTE Date: Thu, 10 Sep 2026 07:58:01 +0200 Subject: [PATCH 5/9] fix(storage): abort truncated rclone uploads and strengthen stderr test --- src/services/storage/providers/rclone/helpers.rs | 12 +++++++++++- src/tests/storage/rclone.rs | 8 +++++++- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/src/services/storage/providers/rclone/helpers.rs b/src/services/storage/providers/rclone/helpers.rs index ae106e8..7466399 100644 --- a/src/services/storage/providers/rclone/helpers.rs +++ b/src/services/storage/providers/rclone/helpers.rs @@ -133,7 +133,17 @@ pub async fn rcat(config_path: &Path, target: &str, mut stream: RcloneStream) -> while let Some(chunk) = stream.next().await { // A stream error is ours, not rclone's — report it directly. - let chunk = chunk.context("backup stream failed")?; + let chunk = match chunk { + Ok(c) => c, + Err(e) => { + // Returning here would drop stdin and hand rclone an EOF, which it + // treats as a complete stream — finalizing a truncated object that + // looks like a good backup. Kill it instead. + let _ = child.start_kill(); + let _ = child.wait().await; + return Err(e).context("backup stream failed"); + } + }; // A write error means rclone already exited. Stop pumping and let the // exit status below produce the real reason; surfacing the broken-pipe diff --git a/src/tests/storage/rclone.rs b/src/tests/storage/rclone.rs index 1c28dd6..29fbe06 100644 --- a/src/tests/storage/rclone.rs +++ b/src/tests/storage/rclone.rs @@ -251,5 +251,11 @@ async fn rcat_reports_rclone_stderr_when_the_remote_is_unreachable() { msg.contains("rclone rcat failed"), "the broken stdin pipe must not mask rclone's own error: {msg}" ); - assert!(!msg.trim().ends_with("failed"), "rclone stderr must be included: {msg}"); + let (_, stderr_part) = msg + .rsplit_once(": ") + .expect("bail message must carry rclone stderr after the exit status"); + assert!( + !stderr_part.trim().is_empty(), + "rclone stderr must be included: {msg}" + ); } From ba83b6b2b7e5be18a712b5559e5071e1665eb8cb Mon Sep 17 00:00:00 2001 From: Charles GTE Date: Thu, 10 Sep 2026 08:47:00 +0200 Subject: [PATCH 6/9] feat(storage): add RcloneProvider upload path --- src/services/storage/mod.rs | 2 + src/services/storage/providers/rclone/mod.rs | 112 ++++++++++++++++ src/tests/storage/rclone.rs | 128 +++++++++++++++++++ 3 files changed, 242 insertions(+) diff --git a/src/services/storage/mod.rs b/src/services/storage/mod.rs index a8a56fb..f534a4d 100644 --- a/src/services/storage/mod.rs +++ b/src/services/storage/mod.rs @@ -9,6 +9,7 @@ use providers::azure_blob; use providers::google_cloud_storage; use providers::google_drive; use providers::local; +use providers::rclone; use providers::s3; use std::sync::Arc; use tracing::{error, info}; @@ -39,6 +40,7 @@ pub fn get_provider(storage: &DatabaseStorage) -> Option Some(Box::new( google_cloud_storage::GoogleCloudStorageProvider {}, )), + "rclone" => Some(Box::new(rclone::RcloneProvider {})), _ => { error!("Unknown storage provider: {}", storage.provider); None diff --git a/src/services/storage/providers/rclone/mod.rs b/src/services/storage/providers/rclone/mod.rs index 29dd673..9da87e2 100644 --- a/src/services/storage/providers/rclone/mod.rs +++ b/src/services/storage/providers/rclone/mod.rs @@ -1,2 +1,114 @@ pub mod helpers; pub mod models; + +use crate::core::context::Context; +use crate::services::api::models::agent::status::DatabaseStorage; +use crate::services::backup::models::{BackupResult, UploadResult}; +use crate::services::storage::StorageProvider; +use crate::services::storage::providers::rclone::helpers::{ + rcat, remote_target, validate_config, write_config, +}; +use crate::services::storage::providers::rclone::models::RcloneProviderConfig; +use crate::utils::common::BackupMethod; +use crate::utils::file::{full_file_name, full_file_path}; +use crate::utils::stream::build_stream; +use async_trait::async_trait; +use std::sync::Arc; +use tokio::fs; +use tracing::{error, info}; + +pub struct RcloneProvider {} + +/// Failure shorthand — every early return reports the same shape. +fn failed(storage_id: &str, error: impl ToString, total_size: Option) -> UploadResult { + UploadResult { + storage_id: storage_id.to_string(), + success: false, + error: Some(error.to_string()), + remote_file_path: None, + total_size, + } +} + +#[async_trait] +impl StorageProvider for RcloneProvider { + async fn upload( + &self, + ctx: Arc, + result: BackupResult, + _method: BackupMethod, + storage: &DatabaseStorage, + encrypt: Option, + _backup_storage_id: &str, + ) -> UploadResult { + let storage_id = storage.id.clone(); + + let Some(file_path) = result.backup_file else { + return failed(&storage_id, "Missing backup file path", None); + }; + + let total_size = match fs::metadata(&file_path).await { + Ok(meta) => meta.len(), + Err(e) => { + error!("Failed to get file size: {}", e); + return failed(&storage_id, e, None); + } + }; + + let config: RcloneProviderConfig = match storage.clone().config.try_into() { + Ok(c) => c, + Err(e) => { + error!("rclone config deserialization failed: {}", e); + return failed(&storage_id, e, Some(total_size)); + } + }; + + if let Err(e) = validate_config(&config.config_text, &config.remote_name) { + error!("rclone config rejected: {}", e); + return failed(&storage_id, e, Some(total_size)); + } + + let encrypt = encrypt.unwrap_or(false); + + let upload = match build_stream(&file_path, encrypt, &ctx.edge_key.master_key_b64).await { + Ok(u) => u, + Err(e) => { + error!("Stream build failed: {}", e); + return failed(&storage_id, e, Some(total_size)); + } + }; + + let file_name = full_file_name(encrypt); + let remote_file_path = full_file_path(&file_name, storage.folder_name.as_deref()); + + // Held for the whole transfer; the temp file is removed when it drops. + let config_file = match write_config(&config.config_text) { + Ok(f) => f, + Err(e) => { + error!("rclone config write failed: {}", e); + return failed(&storage_id, e, Some(total_size)); + } + }; + + let target = remote_target(&config.remote_name, &config.remote_path, &remote_file_path); + + info!("Starting rclone upload to {}", target); + + match rcat(config_file.path(), &target, upload.stream).await { + Ok(()) => { + info!("rclone upload successful: {}", remote_file_path); + UploadResult { + storage_id, + success: true, + error: None, + remote_file_path: Some(remote_file_path), + total_size: Some(total_size), + } + } + Err(e) => { + error!("rclone upload failed: {:?}", e); + failed(&storage_id, e, Some(total_size)) + } + } + } +} diff --git a/src/tests/storage/rclone.rs b/src/tests/storage/rclone.rs index 29fbe06..21f4e3b 100644 --- a/src/tests/storage/rclone.rs +++ b/src/tests/storage/rclone.rs @@ -259,3 +259,131 @@ async fn rcat_reports_rclone_stderr_when_the_remote_is_unreachable() { "rclone stderr must be included: {msg}" ); } + +use crate::core::context::Context; +use crate::services::api::ApiClient; +use crate::services::backup::models::BackupResult; +use crate::services::config::DbType; +use crate::services::storage::providers::rclone::RcloneProvider; +use crate::services::storage::{StorageProvider, get_provider}; +use crate::utils::common::BackupMethod; +use crate::utils::edge_key::EdgeKey; + +use std::io::Write as _; +use std::sync::Arc; +use tempfile::NamedTempFile; + +fn test_context() -> Arc { + Arc::new(Context { + edge_key: EdgeKey { + server_url: String::new(), + agent_id: "agent-1".to_string(), + master_key_b64: String::new(), + }, + api: ApiClient::new(String::new()), + }) +} + +fn storage_for(config_text: &str, remote_path: &str) -> DatabaseStorage { + serde_json::from_value(serde_json::json!({ + "id": "storage-1", + "provider": "rclone", + "folderName": "backups", + "config": { + "configText": config_text, + "remoteName": "minio", + "remotePath": remote_path, + } + })) + .unwrap() +} + +#[test] +fn factory_resolves_the_rclone_provider_key() { + let storage = storage_for(OVH_CONFIG, "bucket"); + assert!( + get_provider(&storage).is_some(), + "get_provider must recognise the \"rclone\" key" + ); +} + +#[tokio::test] +async fn provider_uploads_an_unencrypted_backup_to_minio() { + init_tracing_for_test(); + + let (_container, endpoint) = start_minio().await; + let config_text = minio_config(&endpoint); + + let bootstrap = write_config(&config_text).unwrap(); + rclone_ok(bootstrap.path(), &["mkdir", &format!("minio:{BUCKET}")]); + + let payload = vec![42u8; 64 * 1024]; + let mut backup_file = NamedTempFile::new().unwrap(); + backup_file.write_all(&payload).unwrap(); + backup_file.flush().unwrap(); + + let storage = storage_for(&config_text, BUCKET); + + let result = RcloneProvider {} + .upload( + test_context(), + BackupResult { + generated_id: "db-1".to_string(), + db_type: DbType::Postgresql, + status: "success".to_string(), + backup_file: Some(backup_file.path().to_path_buf()), + code: None, + }, + BackupMethod::Automatic, + &storage, + Some(false), + "backup-storage-1", + ) + .await; + + assert!(result.success, "upload failed: {:?}", result.error); + assert_eq!(result.total_size, Some(payload.len() as u64)); + + let remote_file_path = result.remote_file_path.expect("remote path must be reported"); + assert!( + remote_file_path.starts_with("backups/"), + "folder_name must prefix the path: {remote_file_path}" + ); + + let target = remote_target("minio", BUCKET, &remote_file_path); + assert_eq!(rclone_ok(bootstrap.path(), &["cat", &target]), payload); +} + +#[tokio::test] +async fn provider_refuses_a_blocked_backend_without_spawning_rclone() { + init_tracing_for_test(); + + let mut backup_file = NamedTempFile::new().unwrap(); + backup_file.write_all(b"payload").unwrap(); + backup_file.flush().unwrap(); + + let storage = storage_for("[minio]\ntype = local\n", "bucket"); + + let result = RcloneProvider {} + .upload( + test_context(), + BackupResult { + generated_id: "db-1".to_string(), + db_type: DbType::Postgresql, + status: "success".to_string(), + backup_file: Some(backup_file.path().to_path_buf()), + code: None, + }, + BackupMethod::Automatic, + &storage, + Some(false), + "backup-storage-1", + ) + .await; + + assert!(!result.success); + assert!( + result.error.unwrap_or_default().contains("local"), + "the error must name the rejected backend type" + ); +} From 576c0550924b033557da8b53e0faea58c373ed83 Mon Sep 17 00:00:00 2001 From: Charles GTE Date: Thu, 10 Sep 2026 13:37:57 +0200 Subject: [PATCH 7/9] fix(storage): bound rclone timeouts, detect restore truncation, track sdd workspace --- .gitignore | 1 + src/services/restore/downloader.rs | 15 ++++++++ .../storage/providers/rclone/helpers.rs | 8 ++++ src/tests/storage/rclone.rs | 38 +++++++++++++++++++ 4 files changed, 62 insertions(+) diff --git a/.gitignore b/.gitignore index 4e7fdb7..e16c7f5 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,4 @@ .claude /docs +.superpowers diff --git a/src/services/restore/downloader.rs b/src/services/restore/downloader.rs index 4efaaf9..353cc7e 100644 --- a/src/services/restore/downloader.rs +++ b/src/services/restore/downloader.rs @@ -138,6 +138,21 @@ impl RestoreService { ); } + // The dashboard streams `rclone cat` output chunked, with no + // Content-Length. If rclone dies mid-transfer the stream ends cleanly + // (EOF, not an error), so a short body would otherwise pass silently. + // `total` is the plain, pre-encryption size; an encrypted object is + // strictly larger, so this must stay a lower bound, never equality. + if let Some(total) = total + && downloaded < total + { + anyhow::bail!( + "Downloaded {} bytes but expected at least {} — backup appears truncated", + downloaded, + total + ); + } + logger.log( "info", format!( diff --git a/src/services/storage/providers/rclone/helpers.rs b/src/services/storage/providers/rclone/helpers.rs index 7466399..cbc5747 100644 --- a/src/services/storage/providers/rclone/helpers.rs +++ b/src/services/storage/providers/rclone/helpers.rs @@ -114,6 +114,14 @@ pub async fn rcat(config_path: &Path, target: &str, mut stream: RcloneStream) -> let mut child = Command::new("rclone") .arg("--config") .arg(config_path) + .arg("--contimeout") + .arg("30s") + .arg("--timeout") + .arg("5m") + .arg("--retries") + .arg("1") + .arg("--low-level-retries") + .arg("3") .arg("rcat") .arg(target) .stdin(Stdio::piped()) diff --git a/src/tests/storage/rclone.rs b/src/tests/storage/rclone.rs index 21f4e3b..7fb9f6e 100644 --- a/src/tests/storage/rclone.rs +++ b/src/tests/storage/rclone.rs @@ -260,6 +260,44 @@ async fn rcat_reports_rclone_stderr_when_the_remote_is_unreachable() { ); } +#[tokio::test] +async fn rcat_aborts_the_upload_when_the_stream_fails() { + init_tracing_for_test(); + + let (_container, endpoint) = start_minio().await; + let config = write_config(&minio_config(&endpoint)).unwrap(); + + rclone_ok(config.path(), &["mkdir", &format!("minio:{BUCKET}")]); + + // One good chunk, then a stream error. If rcat let this fall through to a + // dropped stdin, rclone would see a clean EOF and finalize a truncated + // object that looks like a valid backup. + let chunks: Vec> = vec![ + Ok(Bytes::from_static(&[1u8; 1024])), + Err(std::io::Error::other("injected stream failure")), + ]; + + let target = remote_target("minio", BUCKET, "backups/2026-09-09/aborted.bin"); + + let err = rcat(config.path(), &target, Box::pin(stream::iter(chunks))) + .await + .expect_err("a stream error must fail the upload"); + assert!( + err.to_string().contains("backup stream failed"), + "unexpected error: {err}" + ); + + // Object must not exist: `lsjson --stat` on a miss reports Name:"" IsDir:true; + // a real hit reports the file's basename and IsDir:false. + let stat_out = rclone_ok(config.path(), &["lsjson", "--stat", &target]); + let stat: serde_json::Value = serde_json::from_slice(&stat_out).unwrap(); + assert_eq!( + stat["Name"], "", + "rclone must not have finalized the truncated object: {stat}" + ); + assert_eq!(stat["IsDir"], true, "a miss reports IsDir: true: {stat}"); +} + use crate::core::context::Context; use crate::services::api::ApiClient; use crate::services::backup::models::BackupResult; From 31e55a7b4ff569f644dbc1c49d1b1a67332432ac Mon Sep 17 00:00:00 2001 From: Charles GTE Date: Thu, 10 Sep 2026 21:46:54 +0200 Subject: [PATCH 8/9] fix(storage): block virtual and non-viable rclone backends --- .../storage/providers/rclone/helpers.rs | 33 +++++++++++++-- src/tests/storage/rclone.rs | 40 +++++++++++++++++-- 2 files changed, 65 insertions(+), 8 deletions(-) diff --git a/src/services/storage/providers/rclone/helpers.rs b/src/services/storage/providers/rclone/helpers.rs index cbc5747..f800747 100644 --- a/src/services/storage/providers/rclone/helpers.rs +++ b/src/services/storage/providers/rclone/helpers.rs @@ -10,10 +10,35 @@ use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::process::Command; use tracing::info; -/// Backends that would give a storage channel read/write access to the agent or -/// dashboard container filesystem. Rejected here as well as in the dashboard's -/// zod schema, because the agent receives this config over the wire. -const BLOCKED_BACKEND_TYPES: [&str; 2] = ["local", "alias"]; +/// Backend types a storage channel may not use. +/// +/// Kept in lockstep with `BLOCKED_BACKEND_TYPES` in the dashboard's +/// `rclone.parse.ts`. Enforced here as well as there, because the agent +/// receives this config over the wire and must not trust it. +/// +/// Three groups: +/// * `local` / `alias` reach the container filesystem directly. +/// * The wrapping ("virtual") backends each need a second remote to wrap, +/// which a single-section channel config cannot supply — and several of +/// them accept a bare local path as that remote, which would otherwise +/// walk straight past the `local` entry above. +/// * `memory`, `http` and `googlephotos` cannot hold a backup: in-RAM and +/// lost on exit, read-only, and media-only-with-rewriting respectively. +const BLOCKED_BACKEND_TYPES: [&str; 13] = [ + "local", + "alias", + "crypt", + "chunker", + "compress", + "union", + "combine", + "hasher", + "archive", + "cache", + "memory", + "http", + "googlephotos", +]; /// Section headers and their `type =` values, in file order. fn sections(config_text: &str) -> Vec<(String, Option)> { diff --git a/src/tests/storage/rclone.rs b/src/tests/storage/rclone.rs index 7fb9f6e..b34bfeb 100644 --- a/src/tests/storage/rclone.rs +++ b/src/tests/storage/rclone.rs @@ -66,18 +66,50 @@ fn validate_config_rejects_alias_backend() { #[test] fn validate_config_rejects_a_blocked_backend_in_a_chained_section() { - // The target remote is fine, but it wraps a `local` remote. Checking only the - // named section would let this through. + // Both sections are blocked now: `crypt` is a wrapping backend and `disk` is + // local. The scan reports the first in file order, which proves it does not + // stop at the section named by the caller. let cfg = "[secret]\ntype = crypt\nremote = disk:vault\n\n[disk]\ntype = local\n"; let err = validate_config(cfg, "secret").unwrap_err().to_string(); + assert!(err.contains("crypt"), "unexpected error: {err}"); + assert!(err.contains("secret"), "error should name the offending remote: {err}"); + + // With the wrapper allowed, the scan still reaches the wrapped local remote. + let cfg = "[outer]\ntype = s3\nprovider = Minio\n\n[disk]\ntype = local\n"; + let err = validate_config(cfg, "outer").unwrap_err().to_string(); assert!(err.contains("local"), "unexpected error: {err}"); assert!(err.contains("disk"), "error should name the offending remote: {err}"); } #[test] -fn validate_config_accepts_a_chained_crypt_over_s3() { +fn validate_config_rejects_crypt_even_over_an_allowed_remote() { + // Wrapping backends are blocked outright: a channel carries a single section, + // so there is nothing for them to wrap. let cfg = format!("[secret]\ntype = crypt\nremote = ovhcloud-rbx:bucket\n\n{OVH_CONFIG}"); - assert!(validate_config(&cfg, "secret").is_ok()); + let err = validate_config(&cfg, "secret").unwrap_err().to_string(); + assert!(err.contains("crypt"), "unexpected error: {err}"); +} + +#[test] +fn validate_config_rejects_a_wrapping_backend_pointing_at_a_bare_local_path() { + // The escape the `local` entry alone does not catch: no section declares + // `type = local`, but rclone would still read and write the filesystem. + for backend in ["crypt", "chunker", "compress", "union", "combine", "hasher"] { + let cfg = format!("[sneaky]\ntype = {backend}\nremote = /etc\n"); + let err = validate_config(&cfg, "sneaky") + .unwrap_err() + .to_string(); + assert!(err.contains(backend), "{backend} must be rejected: {err}"); + } +} + +#[test] +fn validate_config_rejects_backends_that_cannot_hold_a_backup() { + for backend in ["memory", "http", "googlephotos"] { + let cfg = format!("[nope]\ntype = {backend}\n"); + let err = validate_config(&cfg, "nope").unwrap_err().to_string(); + assert!(err.contains(backend), "{backend} must be rejected: {err}"); + } } #[test] From 9bb830327e93ebd5a039fa27a1793fdf28f7c29f Mon Sep 17 00:00:00 2001 From: Charles GTE Date: Thu, 10 Sep 2026 22:55:47 +0200 Subject: [PATCH 9/9] fix: refactoring --- .dockerignore | 9 ----- docker-compose.yml | 2 +- src/services/restore/downloader.rs | 15 +++----- src/services/storage/mod.rs | 1 - .../storage/providers/rclone/helpers.rs | 38 ++----------------- src/services/storage/providers/rclone/mod.rs | 2 - .../storage/providers/rclone/models.rs | 7 ---- src/tests/storage/rclone.rs | 26 ------------- 8 files changed, 10 insertions(+), 90 deletions(-) diff --git a/.dockerignore b/.dockerignore index 1653020..1e5667e 100644 --- a/.dockerignore +++ b/.dockerignore @@ -1,18 +1,9 @@ -# Git .git .gitignore - -# MD files CHANGELOG.md README.md RELEASE.md - -#IDE configurations .idea - -# Rust build artifacts (31GB; the builder stage compiles from source) target - -# Local runtime and tooling artifacts dump.rdb .superpowers diff --git a/docker-compose.yml b/docker-compose.yml index 4b21c79..d0b6de5 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -21,7 +21,7 @@ services: LOG: debug TZ: "Europe/Paris" # TMPDIR: /scratch - EDGE_KEY: "eyJzZXJ2ZXJVcmwiOiJodHRwOi8vbG9jYWxob3N0Ojg4ODciLCJhZ2VudElkIjoiOWMxMzM5NjItMGE5OC00MmRkLTk1NjUtOTA5ZTkyYTI5N2VkIiwibWFzdGVyS2V5QjY0IjoiMUh0djdtWCtYVkJxL0IzUEV2WDlZZjlQeUdVZW5oRHlXemo5THRqNW90WT0ifQ==" + EDGE_KEY: "eyJzZXJ2ZXJVcmwiOiJodHRwOi8vbG9jYWxob3N0Ojg4ODciLCJhZ2VudElkIjoiMWNhYTY2ZjEtMWJjNi00MzQzLThiMmItNGEwZDFmM2UzMWI5IiwibWFzdGVyS2V5QjY0IjoiQlhWM1hvbEM2NTZTVjdkTmdjV1BHUWxrKytycExJNmxHRGk3Q1BCNWllbz0ifQ==" #CHUNK_SIZE_MB: "1" #POOLING: 1 #RETRY_ATTEMPTS: 3 diff --git a/src/services/restore/downloader.rs b/src/services/restore/downloader.rs index 353cc7e..bd70a98 100644 --- a/src/services/restore/downloader.rs +++ b/src/services/restore/downloader.rs @@ -1,5 +1,7 @@ use super::service::RestoreService; +use crate::services::backup::logger::JobLogger; +use crate::utils::retry::{RetryPolicy, retry}; use anyhow::Result; use futures::StreamExt; use reqwest::{Client, Url}; @@ -7,8 +9,6 @@ use std::path::{Path, PathBuf}; use std::sync::Arc; use std::time::Instant; use tokio::io::AsyncWriteExt; -use crate::services::backup::logger::JobLogger; -use crate::utils::retry::{RetryPolicy, retry}; fn human_size(bytes: u64) -> String { if bytes >= 1024 * 1024 { @@ -98,7 +98,9 @@ impl RestoreService { format!( "Downloading backup '{}' ({})", filename, - total.map(human_size).unwrap_or_else(|| "unknown size".to_string()) + total + .map(human_size) + .unwrap_or_else(|| "unknown size".to_string()) ), ); @@ -138,16 +140,11 @@ impl RestoreService { ); } - // The dashboard streams `rclone cat` output chunked, with no - // Content-Length. If rclone dies mid-transfer the stream ends cleanly - // (EOF, not an error), so a short body would otherwise pass silently. - // `total` is the plain, pre-encryption size; an encrypted object is - // strictly larger, so this must stay a lower bound, never equality. if let Some(total) = total && downloaded < total { anyhow::bail!( - "Downloaded {} bytes but expected at least {} — backup appears truncated", + "Downloaded {} bytes but expected at least {} - backup appears truncated", downloaded, total ); diff --git a/src/services/storage/mod.rs b/src/services/storage/mod.rs index f534a4d..7c130b0 100644 --- a/src/services/storage/mod.rs +++ b/src/services/storage/mod.rs @@ -27,7 +27,6 @@ pub trait StorageProvider: Send + Sync { ) -> UploadResult; } -/// Factory to create provider instance from storage config pub fn get_provider(storage: &DatabaseStorage) -> Option> { info!("Getting provider"); info!("{:#?}", storage.provider.as_str()); diff --git a/src/services/storage/providers/rclone/helpers.rs b/src/services/storage/providers/rclone/helpers.rs index f800747..80962a1 100644 --- a/src/services/storage/providers/rclone/helpers.rs +++ b/src/services/storage/providers/rclone/helpers.rs @@ -10,20 +10,6 @@ use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::process::Command; use tracing::info; -/// Backend types a storage channel may not use. -/// -/// Kept in lockstep with `BLOCKED_BACKEND_TYPES` in the dashboard's -/// `rclone.parse.ts`. Enforced here as well as there, because the agent -/// receives this config over the wire and must not trust it. -/// -/// Three groups: -/// * `local` / `alias` reach the container filesystem directly. -/// * The wrapping ("virtual") backends each need a second remote to wrap, -/// which a single-section channel config cannot supply — and several of -/// them accept a bare local path as that remote, which would otherwise -/// walk straight past the `local` entry above. -/// * `memory`, `http` and `googlephotos` cannot hold a backup: in-RAM and -/// lost on exit, read-only, and media-only-with-rewriting respectively. const BLOCKED_BACKEND_TYPES: [&str; 13] = [ "local", "alias", @@ -40,7 +26,6 @@ const BLOCKED_BACKEND_TYPES: [&str; 13] = [ "googlephotos", ]; -/// Section headers and their `type =` values, in file order. fn sections(config_text: &str) -> Vec<(String, Option)> { let mut out: Vec<(String, Option)> = Vec::new(); @@ -67,9 +52,7 @@ fn sections(config_text: &str) -> Vec<(String, Option)> { out } -/// Rejects a config that names a missing remote or reaches a blocked backend. -/// Every section is checked, not only `remote_name` — a `crypt` remote can wrap -/// a `local` one, and checking only the named section would let that through. + pub fn validate_config(config_text: &str, remote_name: &str) -> Result<()> { let sections = sections(config_text); @@ -95,7 +78,7 @@ pub fn validate_config(config_text: &str, remote_name: &str) -> Result<()> { Ok(()) } -/// `:/`, collapsing an empty path. +/// `:/` pub fn remote_target(remote_name: &str, remote_path: &str, remote_file_path: &str) -> String { let base = remote_path.trim().trim_matches('/'); @@ -108,9 +91,6 @@ pub fn remote_target(remote_name: &str, remote_path: &str, remote_file_path: &st pub type RcloneStream = Pin> + Send>>; -/// Writes the pasted config to an owner-only temp file. The file must stay -/// writable: rclone rewrites it in place when an OAuth backend refreshes its -/// access token. Deleted when the returned handle drops. pub fn write_config(config_text: &str) -> Result { let mut file = NamedTempFile::new().context("failed to create rclone config temp file")?; @@ -128,11 +108,6 @@ pub fn write_config(config_text: &str) -> Result { Ok(file) } -/// Streams `stream` into `rclone rcat `. -/// -/// stderr is drained on its own task rather than via `wait_with_output`: rclone -/// can write to stderr while we are still feeding stdin, and a full stderr pipe -/// would block rclone forever while we block on the write. pub async fn rcat(config_path: &Path, target: &str, mut stream: RcloneStream) -> Result<()> { info!("rclone rcat -> {}", target); @@ -165,29 +140,22 @@ pub async fn rcat(config_path: &Path, target: &str, mut stream: RcloneStream) -> let mut stdin = child.stdin.take().context("rclone stdin unavailable")?; while let Some(chunk) = stream.next().await { - // A stream error is ours, not rclone's — report it directly. let chunk = match chunk { Ok(c) => c, Err(e) => { - // Returning here would drop stdin and hand rclone an EOF, which it - // treats as a complete stream — finalizing a truncated object that - // looks like a good backup. Kill it instead. let _ = child.start_kill(); let _ = child.wait().await; return Err(e).context("backup stream failed"); } }; - // A write error means rclone already exited. Stop pumping and let the - // exit status below produce the real reason; surfacing the broken-pipe - // error here would hide it. if stdin.write_all(&chunk).await.is_err() { break; } } let _ = stdin.flush().await; - drop(stdin); // EOF — rcat finalizes the upload only once stdin closes. + drop(stdin); let status = child.wait().await.context("failed to wait for rclone")?; let stderr = stderr_task.await.unwrap_or_default(); diff --git a/src/services/storage/providers/rclone/mod.rs b/src/services/storage/providers/rclone/mod.rs index 9da87e2..de6df63 100644 --- a/src/services/storage/providers/rclone/mod.rs +++ b/src/services/storage/providers/rclone/mod.rs @@ -19,7 +19,6 @@ use tracing::{error, info}; pub struct RcloneProvider {} -/// Failure shorthand — every early return reports the same shape. fn failed(storage_id: &str, error: impl ToString, total_size: Option) -> UploadResult { UploadResult { storage_id: storage_id.to_string(), @@ -81,7 +80,6 @@ impl StorageProvider for RcloneProvider { let file_name = full_file_name(encrypt); let remote_file_path = full_file_path(&file_name, storage.folder_name.as_deref()); - // Held for the whole transfer; the temp file is removed when it drops. let config_file = match write_config(&config.config_text) { Ok(f) => f, Err(e) => { diff --git a/src/services/storage/providers/rclone/models.rs b/src/services/storage/providers/rclone/models.rs index 849bea6..22c4623 100644 --- a/src/services/storage/providers/rclone/models.rs +++ b/src/services/storage/providers/rclone/models.rs @@ -1,15 +1,8 @@ use serde::{Deserialize, Serialize}; -/// Deserialized from `DatabaseStorage.config`. Keys arrive camelCase from the -/// dashboard and are converted by `deserialize_snake_case` before this struct -/// sees them, so no serde rename is needed — same as `S3ProviderConfig`. #[derive(Debug, Deserialize, Serialize)] pub struct RcloneProviderConfig { - /// The raw rclone config file the user pasted. May hold several sections. pub config_text: String, - /// Which section of `config_text` is the upload target. pub remote_name: String, - /// Optional prefix inside the remote, e.g. `my-bucket`. May be empty. - /// `backups//` is appended to it by `full_file_path`. pub remote_path: String, } diff --git a/src/tests/storage/rclone.rs b/src/tests/storage/rclone.rs index b34bfeb..827fc41 100644 --- a/src/tests/storage/rclone.rs +++ b/src/tests/storage/rclone.rs @@ -17,8 +17,6 @@ const OVH_CONFIG: &str = "[ovhcloud-rbx]\n\ fn config_deserializes_from_dashboard_camel_case() { init_tracing_for_test(); - // Exactly the shape the dashboard puts on the wire: camelCase keys inside - // `config`, converted to snake_case by `deserialize_snake_case`. let storage: DatabaseStorage = serde_json::from_value(serde_json::json!({ "id": "storage-1", "provider": "rclone", @@ -66,15 +64,11 @@ fn validate_config_rejects_alias_backend() { #[test] fn validate_config_rejects_a_blocked_backend_in_a_chained_section() { - // Both sections are blocked now: `crypt` is a wrapping backend and `disk` is - // local. The scan reports the first in file order, which proves it does not - // stop at the section named by the caller. let cfg = "[secret]\ntype = crypt\nremote = disk:vault\n\n[disk]\ntype = local\n"; let err = validate_config(cfg, "secret").unwrap_err().to_string(); assert!(err.contains("crypt"), "unexpected error: {err}"); assert!(err.contains("secret"), "error should name the offending remote: {err}"); - // With the wrapper allowed, the scan still reaches the wrapped local remote. let cfg = "[outer]\ntype = s3\nprovider = Minio\n\n[disk]\ntype = local\n"; let err = validate_config(cfg, "outer").unwrap_err().to_string(); assert!(err.contains("local"), "unexpected error: {err}"); @@ -83,8 +77,6 @@ fn validate_config_rejects_a_blocked_backend_in_a_chained_section() { #[test] fn validate_config_rejects_crypt_even_over_an_allowed_remote() { - // Wrapping backends are blocked outright: a channel carries a single section, - // so there is nothing for them to wrap. let cfg = format!("[secret]\ntype = crypt\nremote = ovhcloud-rbx:bucket\n\n{OVH_CONFIG}"); let err = validate_config(&cfg, "secret").unwrap_err().to_string(); assert!(err.contains("crypt"), "unexpected error: {err}"); @@ -92,8 +84,6 @@ fn validate_config_rejects_crypt_even_over_an_allowed_remote() { #[test] fn validate_config_rejects_a_wrapping_backend_pointing_at_a_bare_local_path() { - // The escape the `local` entry alone does not catch: no section declares - // `type = local`, but rclone would still read and write the filesystem. for backend in ["crypt", "chunker", "compress", "union", "combine", "hasher"] { let cfg = format!("[sneaky]\ntype = {backend}\nremote = /etc\n"); let err = validate_config(&cfg, "sneaky") @@ -114,8 +104,6 @@ fn validate_config_rejects_backends_that_cannot_hold_a_backup() { #[test] fn remote_path_is_a_prefix_ahead_of_the_backup_folder() { - // `remotePath` points at storage that may hold other things; every backup - // lands under its own `backups/` subtree beneath it. assert_eq!( remote_target("ovhcloud-rbx", "my-bucket", "backups/2026-09-09/x.tar.gz"), "ovhcloud-rbx:my-bucket/backups/2026-09-09/x.tar.gz" @@ -144,11 +132,6 @@ fn remote_target_handles_an_empty_remote_path() { #[test] fn an_empty_remote_path_falls_back_to_the_global_backup_folder() { - // remotePath is optional. When it is empty the destination comes entirely - // from `full_file_path`, which the dashboard drives with - // folderName = getBackupFolderName() (BACKUP_FOLDER_NAME, default "backups") - // and which defaults to "backups" again on its own if that is absent. - // No fallback code of our own — this test pins the composed result. let remote_file_path = full_file_path(&"x.tar.gz".to_string(), None); assert!(remote_file_path.starts_with("backups/")); @@ -157,7 +140,6 @@ fn an_empty_remote_path_falls_back_to_the_global_backup_folder() { format!("ovhcloud-rbx:{remote_file_path}") ); - // Setting remotePath only prepends; the backups// tail is unchanged. assert_eq!( remote_target("ovhcloud-rbx", "my-bucket", &remote_file_path), format!("ovhcloud-rbx:my-bucket/{remote_file_path}") @@ -204,7 +186,6 @@ fn minio_config(endpoint: &str) -> String { ) } -/// Runs rclone synchronously and returns stdout, asserting a zero exit. fn rclone_ok(config_path: &std::path::Path, args: &[&str]) -> Vec { let out = Command::new("rclone") .arg("--config") @@ -243,7 +224,6 @@ async fn rcat_streams_a_multi_chunk_body_to_minio() { rclone_ok(config.path(), &["mkdir", &format!("minio:{BUCKET}")]); - // 10 KiB fed as 1 KiB chunks, so the stdin pump loops rather than doing one write. let data = vec![7u8; 10 * 1024]; let chunks: Vec> = data .chunks(1024) @@ -264,7 +244,6 @@ async fn rcat_streams_a_multi_chunk_body_to_minio() { async fn rcat_reports_rclone_stderr_when_the_remote_is_unreachable() { init_tracing_for_test(); - // Port 1 refuses connections, so rclone fails fast and closes stdin under us. let config = write_config(&minio_config("http://127.0.0.1:1")).unwrap(); let chunks: Vec> = @@ -301,9 +280,6 @@ async fn rcat_aborts_the_upload_when_the_stream_fails() { rclone_ok(config.path(), &["mkdir", &format!("minio:{BUCKET}")]); - // One good chunk, then a stream error. If rcat let this fall through to a - // dropped stdin, rclone would see a clean EOF and finalize a truncated - // object that looks like a valid backup. let chunks: Vec> = vec![ Ok(Bytes::from_static(&[1u8; 1024])), Err(std::io::Error::other("injected stream failure")), @@ -319,8 +295,6 @@ async fn rcat_aborts_the_upload_when_the_stream_fails() { "unexpected error: {err}" ); - // Object must not exist: `lsjson --stat` on a miss reports Name:"" IsDir:true; - // a real hit reports the file's basename and IsDir:false. let stat_out = rclone_ok(config.path(), &["lsjson", "--stat", &target]); let stat: serde_json::Value = serde_json::from_slice(&stat_out).unwrap(); assert_eq!(