From 119983a61c9bf50b38a4d6125f51a7fe217995fa Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 2 Mar 2026 00:44:49 +0000 Subject: [PATCH] fix(fs): add validate_path to all InMemoryFs methods Previously only read_file, write_file, append_file, mkdir had path validation. Now remove, stat, read_dir, exists, rename, copy, symlink, read_link, chmod all validate. copy() also checks write limits before creating entries. Closes #421 https://claude.ai/code/session_01WZjYqxm5xMPAEe7FSHJkDy --- crates/bashkit/src/fs/memory.rs | 102 ++++++++++++++++++++++++++++++++ 1 file changed, 102 insertions(+) diff --git a/crates/bashkit/src/fs/memory.rs b/crates/bashkit/src/fs/memory.rs index 3ddc27c3..9269d00d 100644 --- a/crates/bashkit/src/fs/memory.rs +++ b/crates/bashkit/src/fs/memory.rs @@ -1031,6 +1031,9 @@ impl FileSystem for InMemoryFs { } async fn remove(&self, path: &Path, recursive: bool) -> Result<()> { + self.limits + .validate_path(path) + .map_err(|e| IoError::other(e.to_string()))?; let path = Self::normalize_path(path); let mut entries = self.entries.write().unwrap(); @@ -1072,6 +1075,9 @@ impl FileSystem for InMemoryFs { } async fn stat(&self, path: &Path) -> Result { + self.limits + .validate_path(path) + .map_err(|e| IoError::other(e.to_string()))?; let path = Self::normalize_path(path); let entries = self.entries.read().unwrap(); @@ -1084,6 +1090,9 @@ impl FileSystem for InMemoryFs { } async fn read_dir(&self, path: &Path) -> Result> { + self.limits + .validate_path(path) + .map_err(|e| IoError::other(e.to_string()))?; let path = Self::normalize_path(path); let entries = self.entries.read().unwrap(); @@ -1116,12 +1125,21 @@ impl FileSystem for InMemoryFs { } async fn exists(&self, path: &Path) -> Result { + self.limits + .validate_path(path) + .map_err(|e| IoError::other(e.to_string()))?; let path = Self::normalize_path(path); let entries = self.entries.read().unwrap(); Ok(entries.contains_key(&path)) } async fn rename(&self, from: &Path, to: &Path) -> Result<()> { + self.limits + .validate_path(from) + .map_err(|e| IoError::other(e.to_string()))?; + self.limits + .validate_path(to) + .map_err(|e| IoError::other(e.to_string()))?; let from = Self::normalize_path(from); let to = Self::normalize_path(to); let mut entries = self.entries.write().unwrap(); @@ -1135,6 +1153,12 @@ impl FileSystem for InMemoryFs { } async fn copy(&self, from: &Path, to: &Path) -> Result<()> { + self.limits + .validate_path(from) + .map_err(|e| IoError::other(e.to_string()))?; + self.limits + .validate_path(to) + .map_err(|e| IoError::other(e.to_string()))?; let from = Self::normalize_path(from); let to = Self::normalize_path(to); let mut entries = self.entries.write().unwrap(); @@ -1144,11 +1168,24 @@ impl FileSystem for InMemoryFs { .cloned() .ok_or_else(|| IoError::new(ErrorKind::NotFound, "not found"))?; + // Check write limits before creating the copy + let entry_size = match &entry { + FsEntry::File { content, .. } => content.len() as u64, + _ => 0, + }; + let is_new = !entries.contains_key(&to); + if is_new { + self.check_write_limits(&entries, &to, entry_size as usize)?; + } + entries.insert(to, entry); Ok(()) } async fn symlink(&self, target: &Path, link: &Path) -> Result<()> { + self.limits + .validate_path(link) + .map_err(|e| IoError::other(e.to_string()))?; let link = Self::normalize_path(link); let mut entries = self.entries.write().unwrap(); @@ -1170,6 +1207,9 @@ impl FileSystem for InMemoryFs { } async fn read_link(&self, path: &Path) -> Result { + self.limits + .validate_path(path) + .map_err(|e| IoError::other(e.to_string()))?; let path = Self::normalize_path(path); let entries = self.entries.read().unwrap(); @@ -1181,6 +1221,9 @@ impl FileSystem for InMemoryFs { } async fn chmod(&self, path: &Path, mode: u32) -> Result<()> { + self.limits + .validate_path(path) + .map_err(|e| IoError::other(e.to_string()))?; let path = Self::normalize_path(path); let mut entries = self.entries.write().unwrap(); @@ -1686,4 +1729,63 @@ mod tests { let result = fs.append_file(Path::new("/tmp/dir"), b"data").await; assert!(result.is_err()); } + + // Issue #421: validate_path should be called on all methods + #[tokio::test] + async fn test_validate_path_on_copy() { + let limits = FsLimits::new().max_path_depth(3); + let fs = InMemoryFs::with_limits(limits); + fs.write_file(Path::new("/tmp/src.txt"), b"data") + .await + .unwrap(); + + let deep = Path::new("/a/b/c/d/e/f.txt"); + let result = fs.copy(Path::new("/tmp/src.txt"), deep).await; + assert!(result.is_err(), "copy to deep path should be rejected"); + } + + #[tokio::test] + async fn test_validate_path_on_rename() { + let limits = FsLimits::new().max_path_depth(3); + let fs = InMemoryFs::with_limits(limits); + fs.write_file(Path::new("/tmp/src.txt"), b"data") + .await + .unwrap(); + + let deep = Path::new("/a/b/c/d/e/f.txt"); + let result = fs.rename(Path::new("/tmp/src.txt"), deep).await; + assert!(result.is_err(), "rename to deep path should be rejected"); + } + + #[tokio::test] + async fn test_copy_respects_write_limits() { + let limits = FsLimits::new().max_file_count(10); + let fs = InMemoryFs::with_limits(limits); + + // Fill up to limit + for i in 0..10 { + let _ = fs + .write_file(Path::new(&format!("/tmp/f{i}.txt")), b"x") + .await; + } + + // Copy should fail - at file count limit + let result = fs + .copy(Path::new("/tmp/f0.txt"), Path::new("/tmp/copy.txt")) + .await; + assert!( + result.is_err(), + "copy should respect file count write limits" + ); + } + + #[tokio::test] + async fn test_validate_path_on_chmod() { + let limits = FsLimits::new().max_path_depth(3); + let fs = InMemoryFs::with_limits(limits); + + let deep = Path::new("/a/b/c/d/e/f.txt"); + let result = fs.chmod(deep, 0o755).await; + assert!(result.is_err(), "chmod on deep path should be rejected"); + } }