Skip to content

Commit 9548022

Browse files
claudevdavid
authored andcommitted
Add 6 copy/move safety checks to prevent data loss
- Canonicalize paths in recursion check (prevents ".." and symlink bypass) - Pre-flight destination writability check (access W_OK) - Pre-flight disk space validation after scan (statvfs comparison) - Inode identity check prevents copy-over-self via symlinks/hard links - Path/name length validation (255-byte name, 1024-byte path limits) - Special file filtering skips sockets, FIFOs, devices during scan
1 parent bf37b7f commit 9548022

6 files changed

Lines changed: 549 additions & 8 deletions

File tree

apps/desktop/src-tauri/src/file_system/write_operations/copy.rs

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@ use std::time::{Duration, Instant};
1212
#[cfg(target_os = "macos")]
1313
use crate::file_system::macos_copy::{CopyProgressContext, copy_single_file_native, copy_symlink};
1414

15-
use super::helpers::{resolve_conflict, safe_overwrite_file, spawn_async_sync};
15+
use super::helpers::{
16+
is_same_file, resolve_conflict, safe_overwrite_file, spawn_async_sync, validate_disk_space, validate_path_length,
17+
};
1618
use super::scan::{handle_dry_run, scan_sources, take_cached_scan_result};
1719
use super::state::{CopyTransaction, WriteOperationState, update_operation_status};
1820
use super::types::{
@@ -106,6 +108,9 @@ pub(super) fn copy_files_with_progress(
106108
scan_result.total_bytes
107109
);
108110

111+
// Pre-flight disk space check: verify destination has enough free space
112+
validate_disk_space(destination, scan_result.total_bytes)?;
113+
109114
// Phase 2: Copy files in sorted order with rollback support
110115
let mut transaction = CopyTransaction::new();
111116
let mut files_done = 0;
@@ -344,6 +349,9 @@ fn copy_single_file_sorted(
344349
(dest_path.clone(), false)
345350
};
346351

352+
// Validate destination path length limits
353+
validate_path_length(&actual_dest)?;
354+
347355
if needs_safe_overwrite {
348356
if actual_dest.is_dir() {
349357
fs::remove_dir_all(&actual_dest).map_err(|e| WriteOperationError::IoError {
@@ -401,6 +409,20 @@ fn copy_single_file_sorted(
401409
(dest_path.clone(), false)
402410
};
403411

412+
// Validate destination path length limits
413+
validate_path_length(&actual_dest)?;
414+
415+
// Prevent copying a file over itself via symlinks (same inode + device)
416+
if is_same_file(source, &actual_dest) {
417+
log::warn!(
418+
"copy: skipping {}: source and destination resolve to the same file",
419+
source.display()
420+
);
421+
*files_done += 1;
422+
*bytes_done += metadata.len();
423+
return Ok(());
424+
}
425+
404426
// Check cancellation before copy
405427
if state.cancelled.load(Ordering::Relaxed) {
406428
return Err(WriteOperationError::Cancelled {
@@ -554,6 +576,9 @@ pub(super) fn copy_path_recursive(
554576
(dest_path.clone(), false)
555577
};
556578

579+
// Validate destination path length limits
580+
validate_path_length(&actual_dest)?;
581+
557582
// For symlink overwrite, remove the existing symlink/file first (safe since we can recreate)
558583
if needs_safe_overwrite {
559584
if actual_dest.is_dir() {
@@ -610,6 +635,20 @@ pub(super) fn copy_path_recursive(
610635
(dest_path.clone(), false)
611636
};
612637

638+
// Validate destination path length limits
639+
validate_path_length(&actual_dest)?;
640+
641+
// Prevent copying a file over itself via symlinks (same inode + device)
642+
if is_same_file(source, &actual_dest) {
643+
log::warn!(
644+
"copy: skipping {}: source and destination resolve to the same file",
645+
source.display()
646+
);
647+
*files_done += 1;
648+
*bytes_done += metadata.len();
649+
return Ok(());
650+
}
651+
613652
// Check cancellation before copy
614653
if state.cancelled.load(Ordering::Relaxed) {
615654
return Err(WriteOperationError::Cancelled {
@@ -730,6 +769,9 @@ pub(super) fn copy_path_recursive(
730769
apply_to_all_resolution,
731770
)?;
732771
}
772+
} else {
773+
// Skip special files (sockets, FIFOs, char/block devices)
774+
log::warn!("copy: skipping special file: {}", source.display());
733775
}
734776

735777
Ok(())

apps/desktop/src-tauri/src/file_system/write_operations/helpers.rs

Lines changed: 148 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -64,17 +64,160 @@ pub(crate) fn validate_destination_not_inside_source(
6464
sources: &[PathBuf],
6565
destination: &Path,
6666
) -> Result<(), WriteOperationError> {
67+
// Canonicalize destination to resolve symlinks and ".." segments that could
68+
// bypass a naive starts_with check (e.g. /foo/bar/../foo/sub → /foo/sub)
69+
let canonical_dest = destination.canonicalize().unwrap_or_else(|_| destination.to_path_buf());
70+
6771
for source in sources {
68-
if source.is_dir() && destination.starts_with(source) {
69-
return Err(WriteOperationError::DestinationInsideSource {
70-
source: source.display().to_string(),
71-
destination: destination.display().to_string(),
72-
});
72+
if source.is_dir() {
73+
let canonical_source = source.canonicalize().unwrap_or_else(|_| source.to_path_buf());
74+
if canonical_dest.starts_with(&canonical_source) {
75+
return Err(WriteOperationError::DestinationInsideSource {
76+
source: source.display().to_string(),
77+
destination: destination.display().to_string(),
78+
});
79+
}
7380
}
7481
}
7582
Ok(())
7683
}
7784

85+
/// Checks whether the destination directory is writable using access(W_OK).
86+
#[cfg(unix)]
87+
pub(crate) fn validate_destination_writable(destination: &Path) -> Result<(), WriteOperationError> {
88+
use std::ffi::CString;
89+
use std::os::unix::ffi::OsStrExt;
90+
91+
let c_path = CString::new(destination.as_os_str().as_bytes()).map_err(|_| WriteOperationError::IoError {
92+
path: destination.display().to_string(),
93+
message: "Invalid path".to_string(),
94+
})?;
95+
96+
// SAFETY: c_path is a valid null-terminated C string
97+
let result = unsafe { libc::access(c_path.as_ptr(), libc::W_OK) };
98+
if result != 0 {
99+
return Err(WriteOperationError::PermissionDenied {
100+
path: destination.display().to_string(),
101+
message: "Destination folder is not writable. Check folder permissions in Finder.".to_string(),
102+
});
103+
}
104+
Ok(())
105+
}
106+
107+
#[cfg(not(unix))]
108+
pub(crate) fn validate_destination_writable(_destination: &Path) -> Result<(), WriteOperationError> {
109+
Ok(())
110+
}
111+
112+
/// Checks available disk space on the destination volume against required bytes.
113+
/// Uses statvfs on Unix to query free space.
114+
#[cfg(unix)]
115+
pub(crate) fn validate_disk_space(destination: &Path, required_bytes: u64) -> Result<(), WriteOperationError> {
116+
use std::ffi::CString;
117+
use std::mem::MaybeUninit;
118+
use std::os::unix::ffi::OsStrExt;
119+
120+
let c_path = CString::new(destination.as_os_str().as_bytes()).map_err(|_| WriteOperationError::IoError {
121+
path: destination.display().to_string(),
122+
message: "Invalid path".to_string(),
123+
})?;
124+
125+
let mut stat = MaybeUninit::<libc::statvfs>::uninit();
126+
// SAFETY: c_path is a valid null-terminated C string, stat is a valid pointer
127+
let result = unsafe { libc::statvfs(c_path.as_ptr(), stat.as_mut_ptr()) };
128+
if result != 0 {
129+
// Cannot determine space — continue and let the OS report ENOSPC if it happens
130+
return Ok(());
131+
}
132+
133+
// SAFETY: statvfs succeeded, stat is initialized
134+
let stat = unsafe { stat.assume_init() };
135+
// These casts are needed on macOS where f_bavail/f_frsize may not be u64
136+
#[allow(
137+
clippy::unnecessary_cast,
138+
reason = "Required for macOS where statvfs fields are not u64"
139+
)]
140+
let available = stat.f_bavail as u64 * stat.f_frsize as u64;
141+
142+
if required_bytes > available {
143+
// Determine volume name from mount point for a friendlier message
144+
let volume_name = destination
145+
.ancestors()
146+
.find(|p| p.parent().is_some_and(|pp| pp == Path::new("/Volumes")))
147+
.and_then(|p| p.file_name())
148+
.map(|n| n.to_string_lossy().to_string());
149+
150+
return Err(WriteOperationError::InsufficientSpace {
151+
required: required_bytes,
152+
available,
153+
volume_name,
154+
});
155+
}
156+
157+
Ok(())
158+
}
159+
160+
#[cfg(not(unix))]
161+
pub(crate) fn validate_disk_space(_destination: &Path, _required_bytes: u64) -> Result<(), WriteOperationError> {
162+
Ok(())
163+
}
164+
165+
/// Checks if source and destination resolve to the same file (same inode + device).
166+
/// This prevents data loss when copying a file over itself via a symlink.
167+
#[cfg(unix)]
168+
pub(crate) fn is_same_file(source: &Path, destination: &Path) -> bool {
169+
use std::os::unix::fs::MetadataExt;
170+
171+
let src_meta = match fs::metadata(source) {
172+
Ok(m) => m,
173+
Err(_) => return false,
174+
};
175+
let dst_meta = match fs::metadata(destination) {
176+
Ok(m) => m,
177+
Err(_) => return false,
178+
};
179+
180+
src_meta.dev() == dst_meta.dev() && src_meta.ino() == dst_meta.ino()
181+
}
182+
183+
#[cfg(not(unix))]
184+
pub(crate) fn is_same_file(_source: &Path, _destination: &Path) -> bool {
185+
false
186+
}
187+
188+
// ============================================================================
189+
// Path length validation
190+
// ============================================================================
191+
192+
/// Maximum file name length in bytes (APFS/HFS+ limit)
193+
const MAX_NAME_BYTES: usize = 255;
194+
/// Maximum path length in bytes (macOS PATH_MAX)
195+
const MAX_PATH_BYTES: usize = 1024;
196+
197+
/// Validates that a destination path doesn't exceed filesystem name/path length limits.
198+
pub(crate) fn validate_path_length(dest_path: &Path) -> Result<(), WriteOperationError> {
199+
// Check total path length
200+
let path_str = dest_path.as_os_str();
201+
if path_str.len() > MAX_PATH_BYTES {
202+
return Err(WriteOperationError::IoError {
203+
path: dest_path.display().to_string(),
204+
message: format!("Path exceeds maximum length of {} bytes", MAX_PATH_BYTES),
205+
});
206+
}
207+
208+
// Check file name component length
209+
if let Some(name) = dest_path.file_name()
210+
&& name.len() > MAX_NAME_BYTES
211+
{
212+
return Err(WriteOperationError::IoError {
213+
path: dest_path.display().to_string(),
214+
message: format!("File name exceeds maximum length of {} bytes", MAX_NAME_BYTES),
215+
});
216+
}
217+
218+
Ok(())
219+
}
220+
78221
// ============================================================================
79222
// Symlink loop detection
80223
// ============================================================================

apps/desktop/src-tauri/src/file_system/write_operations/mod.rs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,12 @@
44
//! Operations support batch processing (multiple source files) and cancellation.
55
//!
66
//! Safety features:
7+
//! - Path canonicalization to prevent ".." and symlink bypass of recursion checks
8+
//! - Destination writability check before starting
9+
//! - Pre-flight disk space validation after scan
10+
//! - Inode identity check to prevent copy-over-self via symlinks/hard links
11+
//! - Path/name length validation (255-byte name, 1024-byte path)
12+
//! - Special file filtering (skips sockets, FIFOs, devices)
713
//! - macOS copyfile(3) for full metadata preservation (xattrs, ACLs, resource forks)
814
//! - Symlink preservation (not dereferenced)
915
//! - Symlink loop detection to prevent infinite recursion
@@ -28,7 +34,8 @@ use copy::copy_files_with_progress;
2834
use delete::delete_files_with_progress;
2935
#[cfg(not(test))]
3036
use helpers::{
31-
validate_destination, validate_destination_not_inside_source, validate_not_same_location, validate_sources,
37+
validate_destination, validate_destination_not_inside_source, validate_destination_writable,
38+
validate_not_same_location, validate_sources,
3239
};
3340
use move_op::move_files_with_progress;
3441
#[cfg(not(test))]
@@ -51,7 +58,8 @@ pub use types::{
5158
#[cfg(test)]
5259
#[allow(unused_imports, reason = "Re-exports for test modules in file_system")]
5360
pub(crate) use helpers::{
54-
is_same_filesystem, validate_destination, validate_destination_not_inside_source, validate_not_same_location,
61+
is_same_file, is_same_filesystem, validate_destination, validate_destination_not_inside_source,
62+
validate_destination_writable, validate_disk_space, validate_not_same_location, validate_path_length,
5563
validate_sources,
5664
};
5765
#[cfg(test)]
@@ -84,6 +92,7 @@ pub async fn copy_files_start(
8492
// Validate inputs
8593
validate_sources(&sources)?;
8694
validate_destination(&destination)?;
95+
validate_destination_writable(&destination)?;
8796
validate_not_same_location(&sources, &destination)?;
8897
validate_destination_not_inside_source(&sources, &destination)?;
8998

@@ -166,6 +175,7 @@ pub async fn move_files_start(
166175
// Validate inputs
167176
validate_sources(&sources)?;
168177
validate_destination(&destination)?;
178+
validate_destination_writable(&destination)?;
169179
validate_not_same_location(&sources, &destination)?;
170180
validate_destination_not_inside_source(&sources, &destination)?;
171181

apps/desktop/src-tauri/src/file_system/write_operations/scan.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,9 @@ fn scan_preview_recursive(
207207
visited,
208208
)?;
209209
}
210+
} else {
211+
// Skip special files (sockets, FIFOs, char/block devices)
212+
log::warn!("scan_preview: skipping special file: {}", path.display());
210213
}
211214

212215
// Emit progress periodically
@@ -414,6 +417,9 @@ fn scan_path_recursive(
414417
visited,
415418
)?;
416419
}
420+
} else {
421+
// Skip special files (sockets, FIFOs, char/block devices)
422+
log::warn!("scan: skipping special file: {}", path.display());
417423
}
418424

419425
// Emit progress periodically
@@ -623,6 +629,9 @@ fn dry_run_scan_recursive(
623629
visited,
624630
)?;
625631
}
632+
} else {
633+
// Skip special files (sockets, FIFOs, char/block devices)
634+
log::warn!("dry_run_scan: skipping special file: {}", path.display());
626635
}
627636

628637
// Emit progress periodically

0 commit comments

Comments
 (0)