Skip to content

Harden read/copy/move path operations and enforce no-overwrite semantics #1

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
GhaziAlibi merged 3 commits into from
May 23, 2026
Merged

Harden read/copy/move path operations and enforce no-overwrite semantics #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
98c2968
fix: enforce consistent secure read/copy/move semantics
Copilot May 23, 2026
c3c3a27
chore: bump version to 0.3.0
Copilot May 23, 2026
8c3eaa7
chore: fix author name spelling in Cargo.toml
Copilot May 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
[package]
name = "softpath"
version = "0.2.2"
version = "0.3.0"
edition = "2021"
authors = ["ALIBI Ghazu <ghazi.alibi@outlook.fr>"]
authors = ["ALIBI Ghazi <ghazi.alibi@outlook.fr>"]
description = "A human-friendly file and directory path manipulation library for Rust."
license = "MIT"
repository = "https://github.com/GhaziAlibi/softpath"
Expand Down
3 changes: 3 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,9 @@ fn main() -> Result<(), softpath::SoftPathError> {

// Copy it somewhere else
let backup = "~/config/backup/app.json".into_path()?;
if backup.exists()? {
backup.remove()?;
}
config_file.copy_to(&backup)?;

// Create directories as needed
Expand Down
3 changes: 3 additions & 0 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,9 @@
//!
//! // Copy to backup location
//! let backup = backup_path.join("app.json").into_path()?;
//! if backup.exists()? {
//! backup.remove()?;
//! }
//! config_file.copy_to(&backup)?;
//! # Ok(())
//! # }
Expand Down
18 changes: 18 additions & 0 deletions src/ops/implementations/path_impl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ impl PathExt for &Path {
}

fn read_to_string(&self) -> Result<String, SoftPathError> {
crate::utils::check_path_traversal(self)?;
crate::utils::check_symlink_cycles(self)?;
fs::read_to_string(self).map_err(SoftPathError::from)
}

Expand All @@ -47,17 +49,33 @@ impl PathExt for &Path {
}

fn copy_to<P: AsRef<Path>>(&self, dest: P) -> Result<(), SoftPathError> {
crate::utils::check_path_traversal(self)?;
crate::utils::check_symlink_cycles(self)?;
let dest_path = dest.as_ref();
crate::utils::check_path_traversal(dest_path)?;
crate::utils::check_symlink_cycles(dest_path)?;
if dest_path.exists() {
return Err(SoftPathError::Io(std::io::Error::new(
std::io::ErrorKind::AlreadyExists,
format!("Destination already exists: {}", dest_path.display()),
)));
}
fs::copy(self, dest)?;
Ok(())
}

fn move_to<P: AsRef<Path>>(&self, dest: P) -> Result<(), SoftPathError> {
crate::utils::check_path_traversal(self)?;
crate::utils::check_symlink_cycles(self)?;
let dest_path = dest.as_ref();
crate::utils::check_path_traversal(dest_path)?;
crate::utils::check_symlink_cycles(dest_path)?;
if dest_path.exists() {

@chatgpt-codex-connector chatgpt-codex-connector Bot May 23, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Detect existing destination entries without following symlinks

move_to now relies on dest_path.exists() to enforce no-overwrite semantics, but Path::exists() returns false for dangling symlinks (and some metadata errors). In that case this guard is bypassed and fs::rename can still replace the destination path entry, so an existing symlink at dest is overwritten even though this change intends to reject all pre-existing destinations. This affects any call where dest is a broken symlink; use a symlink-aware existence check (for example via symlink_metadata/try_exists handling) before rename.

Useful? React with 👍 / 👎.

return Err(SoftPathError::Io(std::io::Error::new(
std::io::ErrorKind::AlreadyExists,
format!("Destination already exists: {}", dest_path.display()),
)));
}
fs::rename(self, dest)?;
Ok(())
}
Expand Down
18 changes: 18 additions & 0 deletions src/ops/implementations/pathbuf_impl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ impl PathExt for PathBuf {
}

fn read_to_string(&self) -> Result<String, SoftPathError> {
crate::utils::check_path_traversal(self)?;
crate::utils::check_symlink_cycles(self)?;
fs::read_to_string(self).map_err(SoftPathError::from)
}

Expand All @@ -53,17 +55,33 @@ impl PathExt for PathBuf {
}

fn copy_to<P: AsRef<Path>>(&self, dest: P) -> Result<(), SoftPathError> {
crate::utils::check_path_traversal(self)?;
crate::utils::check_symlink_cycles(self)?;
let dest_path = dest.as_ref();
crate::utils::check_path_traversal(dest_path)?;
crate::utils::check_symlink_cycles(dest_path)?;
if dest_path.exists() {
return Err(SoftPathError::Io(std::io::Error::new(
std::io::ErrorKind::AlreadyExists,
format!("Destination already exists: {}", dest_path.display()),
)));
}
fs::copy(self, dest)?;
Ok(())
}

fn move_to<P: AsRef<Path>>(&self, dest: P) -> Result<(), SoftPathError> {
crate::utils::check_path_traversal(self)?;
crate::utils::check_symlink_cycles(self)?;
let dest_path = dest.as_ref();
crate::utils::check_path_traversal(dest_path)?;
crate::utils::check_symlink_cycles(dest_path)?;
if dest_path.exists() {
return Err(SoftPathError::Io(std::io::Error::new(
std::io::ErrorKind::AlreadyExists,
format!("Destination already exists: {}", dest_path.display()),
)));
}
fs::rename(self, dest)?;
Ok(())
}
Expand Down
14 changes: 3 additions & 11 deletions src/ops/implementations/str_impl.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ impl PathExt for &str {

fn read_to_string(&self) -> Result<String, SoftPathError> {
let path = self.into_path()?;
fs::read_to_string(path).map_err(SoftPathError::from)
path.read_to_string()
}

fn write_string(&self, contents: &str) -> Result<(), SoftPathError> {
Expand All @@ -78,20 +78,12 @@ impl PathExt for &str {

fn copy_to<P: AsRef<Path>>(&self, dest: P) -> Result<(), SoftPathError> {
let from = self.into_path()?;
let dest_path = dest.as_ref();
crate::utils::check_path_traversal(dest_path)?;
crate::utils::check_symlink_cycles(dest_path)?;
fs::copy(&from, dest)?;
Ok(())
from.copy_to(dest)
}

fn move_to<P: AsRef<Path>>(&self, dest: P) -> Result<(), SoftPathError> {
let from = self.into_path()?;
let dest_path = dest.as_ref();
crate::utils::check_path_traversal(dest_path)?;
crate::utils::check_symlink_cycles(dest_path)?;
fs::rename(&from, dest)?;
Ok(())
from.move_to(dest)
}

fn is_empty(&self) -> Result<bool, SoftPathError> {
Expand Down
1 change: 1 addition & 0 deletions src/ops/path_ext.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ pub trait PathExt {
/// - The source path does not exist
/// - The destination path already exists
/// - The user lacks permissions
/// - Any parent directories of the destination are missing
fn move_to<P: AsRef<Path>>(&self, dest: P) -> Result<(), SoftPathError>;

/// Returns true if the path points to an empty file or directory.
Expand Down
28 changes: 28 additions & 0 deletions tests/file_operations.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,34 @@ fn test_copy_and_move() {
cleanup_test_dir(&test_dir);
}

#[test]
fn test_copy_and_move_reject_existing_destination() -> Result<(), SoftPathError> {
let test_dir = setup_test_dir();

let source = test_dir.join("source.txt");
source.write_string("source content")?;

let existing_copy_dest = test_dir.join("existing_copy.txt");
existing_copy_dest.write_string("existing content")?;
let copy_err = source.copy_to(&existing_copy_dest).unwrap_err();
assert!(matches!(
copy_err,
SoftPathError::Io(ref e) if e.kind() == std::io::ErrorKind::AlreadyExists
));

let existing_move_dest = test_dir.join("existing_move.txt");
existing_move_dest.write_string("existing content")?;
let move_err = source.move_to(&existing_move_dest).unwrap_err();
assert!(matches!(
move_err,
SoftPathError::Io(ref e) if e.kind() == std::io::ErrorKind::AlreadyExists
));
assert!(source.exists()?);

cleanup_test_dir(&test_dir);
Ok(())
}

#[test]
fn test_is_empty() -> Result<(), SoftPathError> {
let test_dir = setup_test_dir();
Expand Down
9 changes: 9 additions & 0 deletions tests/test_softpath_errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,15 @@ fn test_io_errors() {
}
}

#[test]
fn test_read_to_string_path_traversal_error() {
let malicious_path = "../../../../../../../../../etc/passwd";
assert!(matches!(
malicious_path.read_to_string(),
Err(SoftPathError::PathTraversal(_))
));
}

#[test]
fn test_error_matching_patterns() {
let malicious_path = "../../../../../../../../../etc/passwd";
Expand Down
Loading