Skip to content

Commit 2fd703e

Browse files
committed
fix(processor): ensure correct error on Linux
1 parent 4169478 commit 2fd703e

File tree

3 files changed

+131
-34
lines changed

3 files changed

+131
-34
lines changed

src/processor/action_handler.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,8 @@ pub(crate) fn process_single_action(
9393

9494
// --- Handle Errors from Action Handlers ---
9595
if let Err(e) = result {
96+
// *** DEBUG LOG ***
97+
eprintln!("[DEBUG] Action handler returned error variant: {:?}", e);
9698
eprintln!("Error processing action for '{}': {}", relative_path_str, e);
9799
summary_updater::update_summary_error(summary, e);
98100
}

src/processor/create.rs

Lines changed: 70 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
use crate::core_types::{Action, CreateStatus};
44
use crate::errors::ProcessError;
55
use std::fs;
6+
use std::io; // Import io for ErrorKind
67
use std::path::Path;
78

89
/// Creates or overwrites a file with the provided content.
@@ -19,6 +20,7 @@ pub(crate) fn process_create(
1920
.ok_or_else(|| ProcessError::Internal("Missing content for create action".to_string()))?;
2021

2122
// Ensure parent directory exists and is a directory
23+
// This might return ParentIsNotDirectory if parent exists as file or if creation fails
2224
ensure_parent_directory(resolved_full_path, resolved_base)?;
2325

2426
let mut status = CreateStatus::Created; // Default optimistic status
@@ -54,7 +56,23 @@ pub(crate) fn process_create(
5456

5557
// Write the file content (as bytes to preserve line endings)
5658
fs::write(resolved_full_path, content.as_bytes())
57-
.map_err(|e| ProcessError::Io { source: e })?;
59+
.map_err(|e| {
60+
// Check if the write failed because the parent path component is a file
61+
if e.kind() == io::ErrorKind::NotADirectory {
62+
// Map this specific IO error to our more descriptive error
63+
let parent_path = resolved_full_path.parent().unwrap_or(resolved_full_path).to_path_buf();
64+
// *** DEBUG LOG ***
65+
eprintln!("[DEBUG] fs::write failed with NotADirectory, mapping to ParentIsNotDirectory for path: {}", resolved_full_path.display());
66+
ProcessError::ParentIsNotDirectory {
67+
path: resolved_full_path.to_path_buf(),
68+
parent_path, // Report the parent path
69+
}
70+
} else {
71+
// *** DEBUG LOG ***
72+
eprintln!("[DEBUG] fs::write failed with other IO error: {:?}, mapping to Io for path: {}", e.kind(), resolved_full_path.display());
73+
ProcessError::Io { source: e }
74+
}
75+
})?;
5876

5977
Ok(status)
6078
}
@@ -63,21 +81,59 @@ pub(crate) fn process_create(
6381
/// Also checks if the parent path itself is unexpectedly a file.
6482
fn ensure_parent_directory(target_path: &Path, resolved_base: &Path) -> Result<(), ProcessError> {
6583
if let Some(parent_dir) = target_path.parent() {
66-
if parent_dir == resolved_base || parent_dir.exists() {
67-
// If parent exists, ensure it's a directory
68-
if !parent_dir.is_dir() {
69-
return Err(ProcessError::ParentIsNotDirectory {
70-
path: target_path.to_path_buf(),
71-
parent_path: parent_dir.to_path_buf(),
72-
});
84+
// Avoid checking the base directory itself if it's the parent
85+
if parent_dir == resolved_base || parent_dir.as_os_str().is_empty() {
86+
return Ok(()); // Base directory is guaranteed to exist and be a dir, or path is in root
87+
}
88+
89+
match fs::metadata(parent_dir) {
90+
Ok(metadata) => {
91+
// Parent exists, check if it's a directory
92+
if !metadata.is_dir() {
93+
// *** DEBUG LOG ***
94+
eprintln!("[DEBUG] Parent metadata exists but is not dir, returning ParentIsNotDirectory for parent: {}", parent_dir.display());
95+
return Err(ProcessError::ParentIsNotDirectory {
96+
path: target_path.to_path_buf(),
97+
parent_path: parent_dir.to_path_buf(),
98+
});
99+
}
100+
// Parent exists and is a directory, all good.
101+
}
102+
Err(ref e) if e.kind() == io::ErrorKind::NotFound => {
103+
// Parent does not exist, try to create it
104+
let relative_parent_dir =
105+
parent_dir.strip_prefix(resolved_base).unwrap_or(parent_dir);
106+
println!(" Creating directory: {}", relative_parent_dir.display());
107+
108+
if let Err(create_err) = fs::create_dir_all(parent_dir) {
109+
// Check if the error is specifically "Not a directory"
110+
// This often indicates an intermediate path component was a file during creation attempt.
111+
if create_err.kind() == io::ErrorKind::NotADirectory {
112+
// *** DEBUG LOG ***
113+
eprintln!("[DEBUG] create_dir_all failed with NotADirectory, returning ParentIsNotDirectory for parent: {}", parent_dir.display());
114+
// Map this specific IO error to our more descriptive error
115+
return Err(ProcessError::ParentIsNotDirectory {
116+
path: target_path.to_path_buf(),
117+
// Report the parent directory we *failed* to create
118+
parent_path: parent_dir.to_path_buf(),
119+
});
120+
} else {
121+
// *** DEBUG LOG ***
122+
eprintln!("[DEBUG] create_dir_all failed with other IO error: {:?}, returning Io for parent: {}", create_err.kind(), parent_dir.display());
123+
// Other I/O error during creation
124+
return Err(ProcessError::Io { source: create_err });
125+
}
126+
}
127+
// Creation successful
128+
}
129+
Err(e) => {
130+
// *** DEBUG LOG ***
131+
eprintln!("[DEBUG] fs::metadata failed with other IO error: {:?}, returning Io for parent: {}", e.kind(), parent_dir.display());
132+
// Other error getting metadata (permissions?)
133+
return Err(ProcessError::Io { source: e });
73134
}
74-
} else {
75-
// Parent does not exist, create it
76-
let relative_parent_dir = parent_dir.strip_prefix(resolved_base).unwrap_or(parent_dir);
77-
println!(" Creating directory: {}", relative_parent_dir.display());
78-
fs::create_dir_all(parent_dir).map_err(|e| ProcessError::Io { source: e })?;
79135
}
80136
}
81-
// If no parent (e.g., root directory), assume it's okay.
137+
// If no parent (e.g., file directly in base), assume it's okay.
82138
Ok(())
83139
}

src/processor/safety.rs

Lines changed: 59 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,41 +1,73 @@
11
//! Path safety validation logic.
22
33
use crate::errors::ProcessError;
4-
use std::io::ErrorKind;
4+
use std::fs; // Use fs::metadata
5+
use std::io::ErrorKind; // Import io and 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)
11-
let canonical_base = base_dir
12-
.canonicalize()
13-
.map_err(|e| ProcessError::Io { source: e })?;
12+
let canonical_base = match base_dir.canonicalize() {
13+
Ok(path) => path,
14+
// Handle potential NotADirectory error if base_dir itself is somehow a file (should be caught earlier, but belt-and-suspenders)
15+
Err(e) if e.kind() == ErrorKind::NotADirectory => {
16+
return Err(ProcessError::ParentIsNotDirectory {
17+
// Technically the base itself isn't a dir
18+
path: base_dir.to_path_buf(),
19+
parent_path: base_dir.parent().unwrap_or(base_dir).to_path_buf(),
20+
});
21+
}
22+
Err(e) => return Err(ProcessError::Io { source: e }),
23+
};
1424

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-
})
25+
// Check if the target path *exists* first using metadata.
26+
match fs::metadata(target_path) {
27+
Ok(_) => {
28+
// Target exists. Canonicalize it for the safety check.
29+
match target_path.canonicalize() {
30+
Ok(canonical_target) => {
31+
if canonical_target.starts_with(&canonical_base) {
32+
Ok(()) // Path exists and is safe
33+
} else {
34+
Err(ProcessError::PathNotSafe {
35+
resolved_path: canonical_target,
36+
base_path: canonical_base,
37+
})
38+
}
39+
}
40+
// *** CATCH NotADirectory during canonicalization of EXISTING target ***
41+
Err(e) if e.kind() == ErrorKind::NotADirectory => {
42+
Err(ProcessError::ParentIsNotDirectory {
43+
path: target_path.to_path_buf(),
44+
parent_path: target_path.parent().unwrap_or(target_path).to_path_buf(),
45+
})
46+
}
47+
Err(e) => {
48+
// Error canonicalizing an *existing* path (permissions?)
49+
Err(ProcessError::PathResolution {
50+
path: target_path.to_path_buf(),
51+
details: format!("Failed to canonicalize existing target path: {}", e),
52+
})
53+
}
2654
}
2755
}
2856
Err(ref e) if e.kind() == ErrorKind::NotFound => {
2957
// Target doesn't exist: Check safety based on its intended parent.
3058
check_nonexistent_target_safety(target_path, &canonical_base)
3159
}
32-
Err(e) => {
33-
// Other error during canonicalization (e.g., permission denied)
34-
Err(ProcessError::PathResolution {
60+
// *** CATCH NotADirectory during metadata check (parent is file) ***
61+
Err(ref e) if e.kind() == ErrorKind::NotADirectory => {
62+
Err(ProcessError::ParentIsNotDirectory {
3563
path: target_path.to_path_buf(),
36-
details: format!("Failed to canonicalize target path: {}", e),
64+
parent_path: target_path.parent().unwrap_or(target_path).to_path_buf(),
3765
})
3866
}
67+
Err(e) => {
68+
// Other error getting metadata (permissions?)
69+
Err(ProcessError::Io { source: e }) // Map other metadata errors to IO
70+
}
3971
}
4072
}
4173

@@ -60,6 +92,13 @@ fn check_nonexistent_target_safety(
6092
})
6193
}
6294
}
95+
// *** CATCH NotADirectory during PARENT canonicalization ***
96+
Err(ref parent_err) if parent_err.kind() == ErrorKind::NotADirectory => {
97+
Err(ProcessError::ParentIsNotDirectory {
98+
path: target_path.to_path_buf(), // The target we were trying to create
99+
parent_path: parent.to_path_buf(), // The parent that is not a directory
100+
})
101+
}
63102
Err(ref parent_err) if parent_err.kind() == ErrorKind::NotFound => {
64103
// Parent directory itself doesn't exist. This implies it needs to be
65104
// created. Assume `create_dir_all` will handle safety within the base.

0 commit comments

Comments
 (0)