From e8022cf2e0b94a94fd4e34236e12d9f3a6505c7b Mon Sep 17 00:00:00 2001 From: Olize <53581423+Olize@users.noreply.github.com> Date: Wed, 21 Oct 2026 18:25:09 +0200 Subject: [PATCH] Release MCH v0.2.1 (tested private 23b4b5b6) --- CHANGELOG.md | 10 ++ Cargo.lock | 2 +- Cargo.toml | 4 +- README.md | 7 +- docs/SPECIFICATION.md | 10 +- src/engine.rs | 241 +++++++++++++++++++++++++++++++----------- src/hashing.rs | 16 ++- src/resume.rs | 19 ++-- tests/cli.rs | 20 ++++ 9 files changed, 248 insertions(+), 81 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bc7e1a2..8584217 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,15 @@ # Changelog +## 0.2.1 — 2026-10-21 + +- Enforce no-overwrite at commit time on Linux/Android with RENAME_NOREPLACE, and otherwise safe no-replace hard-link publication. +- Use the original private staging file handle for all copy modes; change file permissions only after publication. +- Sync data before overwriting, published metadata for overwrites/moves and destination directories on Unix overwrites. +- Retry interrupted reads and allocate large progress buffers on the heap. +- Regression tests: late destinations, dangling symlinks, eight simultaneous publishers. +- Sync the published directory after resumed overwrites even without move; regression test checks intact source and matching destination. +- Tests on Termux and Fedora vm01 include forced interruption and verified restart; GitHub CI covers Linux, macOS and Windows. + ## 0.2.0 — 2026-10-21 - Resume staging remains owner-private (0600) until the rename publishes it; final permissions are set on the published inode to keep interruptions resumable. diff --git a/Cargo.lock b/Cargo.lock index d43199e..9296f22 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -331,7 +331,7 @@ checksum = "df1d3c3b53da64cf5760482273a98e575c651a67eec7f77df96b5b642de8f039" [[package]] name = "mch" -version = "0.2.0" +version = "0.2.1" dependencies = [ "anyhow", "blake2", diff --git a/Cargo.toml b/Cargo.toml index a6a0c68..7f7ef77 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "mch" -version = "0.2.0" +version = "0.2.1" edition = "2024" license = "AGPL-3.0-or-later" description = "Mighty Copy with Hash (mch) - copy/move with post-copy verification (default: BLAKE3)." @@ -9,7 +9,7 @@ readme = "README.md" [package.metadata.mch-release] # Explicit review decision; automatic version bump alone must not publish resume. -approved_unix_resume = "v1-single-regular-file" +approved_unix_resume = "v1-single-regular-file-reviewed-0.2.1-hardening" [dependencies] anyhow = "1" diff --git a/README.md b/README.md index ec810e8..7348814 100644 --- a/README.md +++ b/README.md @@ -2,7 +2,7 @@ **MCH** is a Rust command-line tool for copying or moving local files and directory trees with optional post-copy integrity verification. Its default hash is **BLAKE3**. -Current version: **0.2.0**. Licensed under **GNU AGPL-3.0-or-later**. +Current version: **0.2.1**. Licensed under **GNU AGPL-3.0-or-later**. ## Features @@ -14,7 +14,8 @@ Current version: **0.2.0**. Licensed under **GNU AGPL-3.0-or-later**. - Preserve Unix symbolic links as links, including dangling links; links are **not followed** when traversing a directory. - Preview all changes with `--dry-run`. - Refuse to overwrite existing files by default; use `--overwrite` explicitly. -- Stage regular files in the destination directory and rename them into place only after successful verification. +- Enforce a safe, atomic no-replace publication when overwrite is disabled; concurrent writers cannot silently replace a late destination. +- Stage regular files privately in the destination directory and publish only after successful verification, keeping the staging file handle open throughout copying. - With `--move`, remove only **planned source files and subsequently empty planned directories** after successful copy/verification and destination synchronization. A new file in the source tree causes a refusal rather than recursive deletion. - Reject copying a directory into itself, duplicate destination names, destination parent symlinks, and special files (e.g. FIFOs). @@ -104,7 +105,7 @@ mch --resume --progress always large.iso /backup/large.iso **Important: MCH is not a transactional backup system.** - Verification normally reads the operating-system **page cache** after writing; equal hashes do not prove that media/controller data has been durably written or will survive a power loss. -- During `--move` MCH synchronizes staged regular files before publishing and, **on Unix**, synchronizes affected destination directory entries before removing planned source files. Filesystems, controllers and power failures may still behave differently, and MCH is not a transactional backup. On Windows, destination directory persistence is not guaranteed. +- During explicit overwrites and moves, MCH synchronizes staged regular file contents and on Unix also synchronizes affected destination directory entries. Actual power-loss durability still depends on the filesystem and hardware. - MCH checks the planned paths before copying and performs verification before committing each file. It does not take a filesystem snapshot. If sources change during copying, verification may fail; sources must be kept stable for reliable results. - An error partway through an operation may leave some **completed** destination files or newly created directories behind. New directories can temporarily retain default permissions/timestamps until finalization succeeds. Sources are retained if copying or verification fails. It is safe to inspect the destination and retry, using `--overwrite` if appropriate. - For a successful `--move`, original sources are deleted after verification. Deleting originals can itself fail, in which case both copies may remain. Do not assume an all-or-nothing transaction across sources. diff --git a/docs/SPECIFICATION.md b/docs/SPECIFICATION.md index 0d45f9e..108da23 100644 --- a/docs/SPECIFICATION.md +++ b/docs/SPECIFICATION.md @@ -1,4 +1,4 @@ -# Technical specification — MCH 0.2.0 +# Technical specification — MCH 0.2.1 Status: implementation contract, not a formal safety proof. @@ -46,6 +46,14 @@ Preflight errors must leave originals untouched. Failures in the copy phase reta - A kill/power interruption can leave incomplete staging and metadata. A mismatched or missing metadata pair is a deliberate hard error, not silently reset. The source/destination filesystem must provide safe regular-file and rename semantics. - This does **not** defend against hostile races or a same-inode/same-size/same-mtime content mutation, cannot guarantee directory durability on power loss, and does not resume directory trees, multiple files or Windows copies. +## v0.2.1 security patch + +- The preflight destination check is not the final publication decision. When overwriting is disabled, publication must be atomic no-replace: Linux/Android use renameat2(RENAME_NOREPLACE) and a fail-closed hard-link fallback where supported; other platforms use a no-replace hard link or safely refuse publication. +- Normal regular-file copying retains an open private staging file handle instead of reopening the path with fs::copy. Unix staging stays private (0600) until publication; source permissions are then applied via the open handle. +- With explicit overwrite or move, staged bytes and published metadata are synchronized. On Unix, explicit overwrites synchronize destination directories, including resume overwrite without move. +- Hash and progress reads retry Interrupted; the 1 MiB progress buffer resides on the heap. +- These changes do not defend against all hostile parent-directory races or guarantee power-loss durability on all filesystems. + ## Limitations and security boundaries - No filesystem snapshots: sources should remain unchanged while MCH runs. diff --git a/src/engine.rs b/src/engine.rs index c2f8db0..586c9ed 100644 --- a/src/engine.rs +++ b/src/engine.rs @@ -471,6 +471,60 @@ fn new_temp_path(destination: &Path) -> Result { )) } +// Enforce no-overwrite *at publication*, not only with a preflight check. +// Creating the final hard link is atomic and fails if any inode (including a +// dangling symlink) appeared at the destination. Both names share one inode +// briefly, then the temporary name is removed. Never fall back to rename: +// rename would silently replace a concurrently created destination. +pub(crate) fn publish_stage(stage: &Path, destination: &Path, overwrite: bool) -> Result<()> { + if !overwrite { + #[cfg(any(target_os = "linux", target_os = "android"))] + { + use std::ffi::CString; + use std::os::unix::ffi::OsStrExt; + let src = CString::new(stage.as_os_str().as_bytes()) + .context("temporary path contains a NUL")?; + let dst = CString::new(destination.as_os_str().as_bytes()) + .context("destination path contains a NUL")?; + // Native atomic move with RENAME_NOREPLACE. This also handles + // Android environments that prohibit owner-created hard links. + let rc = unsafe { + libc::syscall( + libc::SYS_renameat2, + libc::AT_FDCWD, + src.as_ptr(), + libc::AT_FDCWD, + dst.as_ptr(), + libc::RENAME_NOREPLACE, + ) + }; + if rc == 0 { + return Ok(()); + } + let error = std::io::Error::last_os_error(); + // On older kernels or unsupported filesystems, try an equally + // no-replace link operation. Never fall back to fs::rename. + if !matches!( + error.raw_os_error(), + Some(libc::ENOSYS | libc::EINVAL | libc::EOPNOTSUPP) + ) { + return Err(error) + .with_context(|| format!("no-replace publish {}", destination.display())); + } + } + fs::hard_link(stage, destination).with_context(|| { + format!( + "destination appeared or filesystem does not support safe no-replace publish: {}", + destination.display() + ) + })?; + fs::remove_file(stage) + .with_context(|| format!("remove published temporary name {}", stage.display()))?; + return Ok(()); + } + atomic_replace(stage, destination) +} + pub(crate) fn atomic_replace(stage: &Path, destination: &Path) -> Result<()> { #[cfg(windows)] { @@ -509,21 +563,24 @@ fn process_file(cli: &Cli, task: &FileTask, progress: Option<&Progress>) -> Resu check_destination_file(&task.source, &task.destination, cli.overwrite)?; let temporary = new_temp_path(&task.destination)?; - match task.kind { + let (staged_handle, original_permissions) = match task.kind { Kind::Regular => { - // Reserving exclusively makes stale temporary files non-destructive. + // Exclusive creation keeps stale names non-destructive; all writes + // use the same open file descriptor (never reopen by path). let mut staged = reserve_private_stage(&temporary.0)?; + let mut source = fs::File::open(&task.source) + .with_context(|| format!("open source {}", task.source.display()))?; + let original_permissions = source.metadata()?.permissions(); if let Some(progress) = progress { - // Count bytes only when progress is visible. Streaming is needed - // so one large file can report progress before completion. - let mut source = fs::File::open(&task.source) - .with_context(|| format!("open source {}", task.source.display()))?; - let source_permissions = source.metadata()?.permissions(); - let mut buffer = [0_u8; 1024 * 1024]; + let mut buffer = vec![0_u8; 1024 * 1024]; loop { - let size = source - .read(&mut buffer) - .with_context(|| format!("read source {}", task.source.display()))?; + let size = loop { + match source.read(&mut buffer) { + Err(error) if error.kind() == ErrorKind::Interrupted => continue, + result => break result, + } + } + .with_context(|| format!("read source {}", task.source.display()))?; if size == 0 { break; } @@ -532,15 +589,10 @@ fn process_file(cli: &Cli, task: &FileTask, progress: Option<&Progress>) -> Resu .with_context(|| format!("write staged file {}", temporary.0.display()))?; progress.add_bytes(size as u64); } - drop(staged); - drop(source); - fs::set_permissions(&temporary.0, source_permissions) - .with_context(|| format!("copy file permissions {}", temporary.0.display()))?; } else { - // Keep platform-optimized filesystem copying (e.g. copy_file_range) - // when progress is off; do not incur a streaming performance cost. - drop(staged); - fs::copy(&task.source, &temporary.0).with_context(|| { + // std::io::copy specializes file-to-file transfers where the + // platform permits; crucially it uses our private open handle. + std::io::copy(&mut source, &mut staged).with_context(|| { format!( "copy {} -> {}", task.source.display(), @@ -548,6 +600,7 @@ fn process_file(cli: &Cli, task: &FileTask, progress: Option<&Progress>) -> Resu ) })?; } + drop(source); if !cli.no_hash { let original_hash = hash_file(&task.source, cli.hash)?; @@ -561,6 +614,7 @@ fn process_file(cli: &Cli, task: &FileTask, progress: Option<&Progress>) -> Resu ); } } + (Some(staged), Some(original_permissions)) } Kind::Symlink => { #[cfg(unix)] @@ -571,11 +625,12 @@ fn process_file(cli: &Cli, task: &FileTask, progress: Option<&Progress>) -> Resu if !cli.no_hash && fs::read_link(&temporary.0)? != link { bail!("symlink verification FAILED: {}", task.source.display()); } + (None, None) } #[cfg(not(unix))] bail!("symlink copies are unsupported on this platform"); } - } + }; // Apply times to the temporary object before publishing it atomically. // Symlink times must never be applied through the link to its target. @@ -593,56 +648,48 @@ fn process_file(cli: &Cli, task: &FileTask, progress: Option<&Progress>) -> Resu } } - // For a move, flush the complete staged file AND its metadata before - // publishing. Hashing may have read cached data, not durable storage. - if cli.move_mode && matches!(task.kind, Kind::Regular) { - #[cfg(not(windows))] - fs::File::open(&temporary.0) - .with_context(|| format!("open staged data for sync {}", temporary.0.display()))? + // Keep the original private staging handle until after publication. + // Flush verified bytes before any destructive replacement or move, + // including on Windows. Ordinary copies retain the faster default. + if (cli.move_mode || cli.overwrite) + && let Some(staged) = &staged_handle + { + staged .sync_all() .with_context(|| format!("sync staged data {}", temporary.0.display()))?; - #[cfg(windows)] - { - // Windows FlushFileBuffers requires a GENERIC_WRITE file handle; - // sync_all on File::open (read-only) returns AccessDenied. - // fs::copy may also have marked the staged file read-only. - let permissions = fs::metadata(&temporary.0)?.permissions(); - if permissions.readonly() { - let mut writable = permissions.clone(); - // On Windows this toggles FILE_ATTRIBUTE_READONLY only; - // Unix executable-bit concerns from this Clippy lint do not apply. - #[allow(clippy::permissions_set_readonly_false)] - writable.set_readonly(false); - fs::set_permissions(&temporary.0, writable)?; - } - let sync_result = (|| -> Result<()> { - let staged = fs::OpenOptions::new() - .write(true) - .open(&temporary.0) - .with_context(|| { - format!( - "open writable staged data for sync {}", - temporary.0.display() - ) - })?; - if permissions.readonly() { - // Restore the original source's readonly flag before sync. - fs::set_permissions(&temporary.0, permissions.clone())?; - } - staged - .sync_all() - .with_context(|| format!("sync staged data {}", temporary.0.display())) - })(); - if sync_result.is_err() && permissions.readonly() { - let _ = fs::set_permissions(&temporary.0, permissions); - } - sync_result?; - } } // Recheck before committing: the target might have changed during a long copy. check_destination_file(&task.source, &task.destination, cli.overwrite)?; - atomic_replace(&temporary.0, &task.destination)?; + publish_stage(&temporary.0, &task.destination, cli.overwrite)?; + // Do not relax 0600 while staging. Apply original permissions through + // the still-open inode only after it has been safely published. + if let (Some(staged), Some(permissions)) = (&staged_handle, original_permissions) { + staged.set_permissions(permissions).with_context(|| { + format!( + "apply published file permissions {}", + task.destination.display() + ) + })?; + if cli.move_mode || cli.overwrite { + staged.sync_all().with_context(|| { + format!( + "sync published file metadata {}", + task.destination.display() + ) + })?; + } + } + + // For a destructive overwrite the rename itself must be synchronized on + // Unix after data and metadata; otherwise power loss can undo the rename. + #[cfg(unix)] + if cli.overwrite { + fs::File::open(parent) + .with_context(|| format!("open published directory {}", parent.display()))? + .sync_all() + .with_context(|| format!("sync published directory {}", parent.display()))?; + } let label = if matches!(task.kind, Kind::Symlink) { "SYMLINK" @@ -984,6 +1031,72 @@ mod move_safety_tests { use super::*; use clap::Parser; + #[test] + fn publication_without_overwrite_refuses_a_new_target() { + let t = tempfile::tempdir().unwrap(); + let stage = t.path().join("staged"); + let destination = t.path().join("new-target"); + fs::write(&stage, b"new copy").unwrap(); + fs::write(&destination, b"precious other data").unwrap(); + assert!(publish_stage(&stage, &destination, false).is_err()); + assert_eq!(fs::read(&destination).unwrap(), b"precious other data"); + assert_eq!(fs::read(&stage).unwrap(), b"new copy"); + } + + #[cfg(unix)] + #[test] + fn publication_without_overwrite_refuses_dangling_symlink_target() { + use std::os::unix::fs::symlink; + let t = tempfile::tempdir().unwrap(); + let stage = t.path().join("staged"); + let destination = t.path().join("new-target"); + fs::write(&stage, b"new copy").unwrap(); + symlink(t.path().join("missing"), &destination).unwrap(); + assert!(publish_stage(&stage, &destination, false).is_err()); + assert!( + fs::symlink_metadata(&destination) + .unwrap() + .file_type() + .is_symlink() + ); + assert_eq!(fs::read(&stage).unwrap(), b"new copy"); + } + + #[test] + fn concurrent_no_overwrite_publications_have_one_winner() { + use std::sync::{Arc, Barrier}; + let t = tempfile::tempdir().unwrap(); + let target = t.path().join("shared-destination"); + let num = 8; + let barrier = Arc::new(Barrier::new(num)); + let mut workers = Vec::new(); + for id in 0..num { + let path = t.path().join(format!("stage-{id}")); + fs::write(&path, format!("payload-{id}")).unwrap(); + let target = target.clone(); + let barrier = Arc::clone(&barrier); + workers.push(std::thread::spawn(move || { + barrier.wait(); + let result = publish_stage(&path, &target, false); + (id, result.is_ok()) + })); + } + let results: Vec<_> = workers + .into_iter() + .map(|worker| worker.join().unwrap()) + .collect(); + let winners: Vec<_> = results.iter().filter(|(_, ok)| *ok).collect(); + assert_eq!( + winners.len(), + 1, + "exactly one simultaneous publisher must win" + ); + assert_eq!( + fs::read_to_string(target).unwrap(), + format!("payload-{}", winners[0].0) + ); + } + #[cfg(unix)] #[test] fn staged_regular_file_is_private_from_creation() { diff --git a/src/hashing.rs b/src/hashing.rs index eef60b6..13b3774 100644 --- a/src/hashing.rs +++ b/src/hashing.rs @@ -7,6 +7,16 @@ use std::fs::File; use std::io::Read; use std::path::Path; +// An interrupted read is not EOF or corrupted data; try the same read again. +fn read_retry(file: &mut File, buffer: &mut [u8]) -> std::io::Result { + loop { + match file.read(buffer) { + Err(error) if error.kind() == std::io::ErrorKind::Interrupted => continue, + result => return result, + } + } +} + pub(crate) fn hash_file(path: &Path, algorithm: HashAlg) -> Result { let mut file = File::open(path).with_context(|| format!("open for hashing: {}", path.display()))?; @@ -16,7 +26,7 @@ pub(crate) fn hash_file(path: &Path, algorithm: HashAlg) -> Result { ($type:ty) => {{ let mut hasher = <$type>::new(); loop { - let n = file.read(&mut buffer)?; + let n = read_retry(&mut file, &mut buffer)?; if n == 0 { break; } @@ -30,7 +40,7 @@ pub(crate) fn hash_file(path: &Path, algorithm: HashAlg) -> Result { HashAlg::Blake3 => { let mut hasher = blake3::Hasher::new(); loop { - let n = file.read(&mut buffer)?; + let n = read_retry(&mut file, &mut buffer)?; if n == 0 { break; } @@ -47,7 +57,7 @@ pub(crate) fn hash_file(path: &Path, algorithm: HashAlg) -> Result { HashAlg::Xxh3 => { let mut hasher = xxhash_rust::xxh3::Xxh3::new(); loop { - let n = file.read(&mut buffer)?; + let n = read_retry(&mut file, &mut buffer)?; if n == 0 { break; } diff --git a/src/resume.rs b/src/resume.rs index 20eaaff..e53aa7b 100644 --- a/src/resume.rs +++ b/src/resume.rs @@ -390,7 +390,7 @@ mod unix { stage.sync_all().context("sync private staged metadata")?; crate::engine::check_destination_file(source, destination, cli.overwrite)?; inode_unchanged(&stage, &partial_path)?; - crate::engine::atomic_replace(&partial_path, destination) + crate::engine::publish_stage(&partial_path, destination, cli.overwrite) .context("publish fully verified resumed copy")?; // The inode is now the FINAL destination, not a resumable .part. // Set permissions via the already-open handle, avoiding a pathname @@ -402,21 +402,26 @@ mod unix { .sync_all() .context("sync final resumed file permissions")?; drop(stage); - if cli.move_mode { - // A synced *file* alone is not enough to durably record the rename. - // Never remove the only source until the destination directory's - // new entry is synced. This requires a filesystem that supports it. + // A synced file alone does not durably record a published directory + // entry. Sync the directory on every overwrite, even without --move. + // Moving also requires the sync before the source may be deleted. + if cli.move_mode || cli.overwrite { fs::File::open(parent) .with_context(|| { - format!("open destination directory for sync: {}", parent.display()) + format!( + "open resumed destination directory for sync: {}", + parent.display() + ) })? .sync_all() .with_context(|| { format!( - "sync resumed destination directory before source deletion: {}", + "sync published resumed destination directory: {}", parent.display() ) })?; + } + if cli.move_mode { require_original_source(source, &src, &initial_meta) .context("source changed before resumed move cleanup")?; } diff --git a/tests/cli.rs b/tests/cli.rs index f617ece..4768c63 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -1065,6 +1065,26 @@ fn resume_existing_destination_needs_overwrite_and_move_removes_only_after_hash( assert_eq!(fs::read(&dst).unwrap(), b"verified copied content"); } +#[cfg(unix)] +#[test] +fn resume_overwrite_without_move_keeps_source_and_replaces_target() { + let t = TempDir::new().unwrap(); + let src = t.path().join("source"); + let dst = t.path().join("destination"); + fs::write(&src, b"verified replacement for resume overwrite").unwrap(); + fs::write(&dst, b"old target must be replaced").unwrap(); + let part = seed_resume_partial(&src, &dst, 12); + assert_success(mch(&[ + "--resume", + "--overwrite", + src.to_str().unwrap(), + dst.to_str().unwrap(), + ])); + assert!(!part.exists()); + assert_eq!(fs::read(&src).unwrap(), fs::read(&dst).unwrap()); + assert!(src.exists(), "non-move overwrite must never delete source"); +} + #[test] fn resume_rejects_unsupported_flags_and_dry_run_does_not_write() { let t = TempDir::new().unwrap();