Skip to content

Commit 89ecc55

Browse files
committed
fix(processor): ensure correct error for parent-is-file on Linux
The `ensure_path_safe` function previously called `canonicalize()` on the target path directly. On Linux, this fails early with a "Not a directory" error if the target path doesn't exist but its parent path exists and is a file. This prevented the intended `ParentIsNotDirectory` error from being generated during the create operation, leading to an incorrect `failed_io` summary count instead of `failed_parent_isdir`. This commit modifies `ensure_path_safe` to first check if the target path exists using `fs::metadata()`. - If the target exists, it's canonicalized directly for the safety check. - If the target does not exist, the safety check relies on canonicalizing the *parent* path (`check_nonexistent_target_safety`), deferring the specific check for the parent's type to the `create::ensure_parent_directory` function. This ensures that the correct `ProcessError::ParentIsNotDirectory` is raised and reported in the summary when attempting to create a file where the parent path exists but is a file, resolving the test failure on Linux.
1 parent 4169478 commit 89ecc55

File tree

1 file changed

+26
-17
lines changed

1 file changed

+26
-17
lines changed

src/processor/safety.rs

Lines changed: 26 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,49 @@
11
//! Path safety validation logic.
22
33
use crate::errors::ProcessError;
4+
use std::fs; // Use fs::metadata
45
use std::io::ErrorKind;
56
use std::path::Path;
67

78
/// Checks if the target path is safely within the base directory.
8-
/// Canonicalizes both paths for reliable comparison.
9+
/// Canonicalizes paths for reliable comparison.
910
pub(crate) fn ensure_path_safe(base_dir: &Path, target_path: &Path) -> Result<(), ProcessError> {
1011
// Canonicalize base directory (must succeed as it's resolved in process_actions)
1112
let canonical_base = base_dir
1213
.canonicalize()
1314
.map_err(|e| ProcessError::Io { source: e })?;
1415

15-
// Attempt to canonicalize the target path.
16-
match target_path.canonicalize() {
17-
Ok(canonical_target) => {
18-
// If target exists and canonicalizes, check if it starts with the base
19-
if canonical_target.starts_with(&canonical_base) {
20-
Ok(()) // Path is safe
21-
} else {
22-
Err(ProcessError::PathNotSafe {
23-
resolved_path: canonical_target,
24-
base_path: canonical_base,
25-
})
16+
// Check if the target path *exists* first using metadata.
17+
match fs::metadata(target_path) {
18+
Ok(_) => {
19+
// Target exists. Canonicalize it for the safety check.
20+
match target_path.canonicalize() {
21+
Ok(canonical_target) => {
22+
if canonical_target.starts_with(&canonical_base) {
23+
Ok(()) // Path exists and is safe
24+
} else {
25+
Err(ProcessError::PathNotSafe {
26+
resolved_path: canonical_target,
27+
base_path: canonical_base,
28+
})
29+
}
30+
}
31+
Err(e) => {
32+
// Error canonicalizing an *existing* path (permissions?)
33+
Err(ProcessError::PathResolution {
34+
path: target_path.to_path_buf(),
35+
details: format!("Failed to canonicalize existing target path: {}", e),
36+
})
37+
}
2638
}
2739
}
2840
Err(ref e) if e.kind() == ErrorKind::NotFound => {
2941
// Target doesn't exist: Check safety based on its intended parent.
3042
check_nonexistent_target_safety(target_path, &canonical_base)
3143
}
3244
Err(e) => {
33-
// Other error during canonicalization (e.g., permission denied)
34-
Err(ProcessError::PathResolution {
35-
path: target_path.to_path_buf(),
36-
details: format!("Failed to canonicalize target path: {}", e),
37-
})
45+
// Other error getting metadata (permissions?)
46+
Err(ProcessError::Io { source: e }) // Map other metadata errors to IO
3847
}
3948
}
4049
}

0 commit comments

Comments
 (0)