From 0f209ff2192c1d21d9942f3ad6d7bccbf02d234f Mon Sep 17 00:00:00 2001 From: Mykhailo Chalyi Date: Tue, 7 Apr 2026 05:42:50 +0000 Subject: [PATCH] fix(vfs): handle ./ prefix in path resolution Normalize paths with `.` and `..` components in: - Interpreter::resolve_path (used by redirections, test builtin, script exec) - Interpreter test-command inline resolve lambda - Glob expand_glob directory resolution - PosixFs (all FileSystem trait methods) Closes #1114 --- crates/bashkit/src/fs/posix.rs | 109 +++++++++++++++++----- crates/bashkit/src/interpreter/glob.rs | 2 +- crates/bashkit/src/interpreter/mod.rs | 12 ++- crates/bashkit/src/lib.rs | 123 +++++++++++++++++++++++++ 4 files changed, 219 insertions(+), 27 deletions(-) diff --git a/crates/bashkit/src/fs/posix.rs b/crates/bashkit/src/fs/posix.rs index 9a751b1f..33f128ee 100644 --- a/crates/bashkit/src/fs/posix.rs +++ b/crates/bashkit/src/fs/posix.rs @@ -53,6 +53,7 @@ use std::sync::Arc; use super::backend::FsBackend; use super::limits::{FsLimits, FsUsage}; +use super::normalize_path; use super::traits::{DirEntry, FileSystem, FileSystemExt, Metadata, fs_errors}; use crate::error::Result; @@ -100,6 +101,11 @@ impl PosixFs { &self.backend } + /// Normalize a path for consistent lookups. + fn normalize(path: &Path) -> PathBuf { + normalize_path(path) + } + /// Check if parent directory exists. async fn check_parent_exists(&self, path: &Path) -> Result<()> { if let Some(parent) = path.parent() @@ -116,43 +122,47 @@ impl PosixFs { #[async_trait] impl FileSystem for PosixFs { async fn read_file(&self, path: &Path) -> Result> { + let path = Self::normalize(path); // Check if it's a directory - if let Ok(meta) = self.backend.stat(path).await + if let Ok(meta) = self.backend.stat(&path).await && meta.file_type.is_dir() { return Err(fs_errors::is_a_directory()); } - self.backend.read(path).await + self.backend.read(&path).await } async fn write_file(&self, path: &Path, content: &[u8]) -> Result<()> { + let path = Self::normalize(path); // Check parent exists - self.check_parent_exists(path).await?; + self.check_parent_exists(&path).await?; // Check if path is a directory - if let Ok(meta) = self.backend.stat(path).await + if let Ok(meta) = self.backend.stat(&path).await && meta.file_type.is_dir() { return Err(fs_errors::is_a_directory()); } - self.backend.write(path, content).await + self.backend.write(&path, content).await } async fn append_file(&self, path: &Path, content: &[u8]) -> Result<()> { + let path = Self::normalize(path); // Check if path is a directory - if let Ok(meta) = self.backend.stat(path).await + if let Ok(meta) = self.backend.stat(&path).await && meta.file_type.is_dir() { return Err(fs_errors::is_a_directory()); } - self.backend.append(path, content).await + self.backend.append(&path, content).await } async fn mkdir(&self, path: &Path, recursive: bool) -> Result<()> { + let path = Self::normalize(path); // Check if something already exists at this path - if let Ok(meta) = self.backend.stat(path).await { + if let Ok(meta) = self.backend.stat(&path).await { if meta.file_type.is_dir() { // Directory exists if recursive { @@ -181,58 +191,70 @@ impl FileSystem for PosixFs { } } else { // Non-recursive: parent must exist - self.check_parent_exists(path).await?; + self.check_parent_exists(&path).await?; } - self.backend.mkdir(path, recursive).await + self.backend.mkdir(&path, recursive).await } async fn remove(&self, path: &Path, recursive: bool) -> Result<()> { - self.backend.remove(path, recursive).await + let path = Self::normalize(path); + self.backend.remove(&path, recursive).await } async fn stat(&self, path: &Path) -> Result { - self.backend.stat(path).await + let path = Self::normalize(path); + self.backend.stat(&path).await } async fn read_dir(&self, path: &Path) -> Result> { + let path = Self::normalize(path); // Check if it's actually a directory - if let Ok(meta) = self.backend.stat(path).await + if let Ok(meta) = self.backend.stat(&path).await && !meta.file_type.is_dir() { return Err(fs_errors::not_a_directory()); } - self.backend.read_dir(path).await + self.backend.read_dir(&path).await } async fn exists(&self, path: &Path) -> Result { - self.backend.exists(path).await + let path = Self::normalize(path); + self.backend.exists(&path).await } async fn rename(&self, from: &Path, to: &Path) -> Result<()> { - self.backend.rename(from, to).await + let from = Self::normalize(from); + let to = Self::normalize(to); + self.backend.rename(&from, &to).await } async fn copy(&self, from: &Path, to: &Path) -> Result<()> { + let from = Self::normalize(from); + let to = Self::normalize(to); // Check source is not a directory - if let Ok(meta) = self.backend.stat(from).await + if let Ok(meta) = self.backend.stat(&from).await && meta.file_type.is_dir() { return Err(IoError::other("cannot copy directory").into()); } - self.backend.copy(from, to).await + self.backend.copy(&from, &to).await } async fn symlink(&self, target: &Path, link: &Path) -> Result<()> { - self.backend.symlink(target, link).await + let target = Self::normalize(target); + let link = Self::normalize(link); + self.backend.symlink(&target, &link).await } async fn read_link(&self, path: &Path) -> Result { - self.backend.read_link(path).await + let path = Self::normalize(path); + self.backend.read_link(&path).await } async fn chmod(&self, path: &Path, mode: u32) -> Result<()> { - self.backend.chmod(path, mode).await + let path = Self::normalize(path); + self.backend.chmod(&path, mode).await } } @@ -295,4 +317,49 @@ mod tests { let result = fs.mkdir(Path::new("/tmp/testfile"), false).await; assert!(result.is_err()); } + + #[tokio::test] + async fn test_posix_normalize_dot_slash_prefix() { + // Issue #1114: paths with ./ prefix should resolve correctly + let fs = InMemoryFs::new(); + + // Create /tmp/dir and a file + fs.mkdir(Path::new("/tmp/dir"), true).await.unwrap(); + fs.write_file(Path::new("/tmp/dir/file.txt"), b"content") + .await + .unwrap(); + + // Access via ./ style path (as if cwd.join("./file.txt")) + let dot_path = Path::new("/tmp/dir/./file.txt"); + assert!( + fs.exists(dot_path).await.unwrap(), + "exists with ./ should work" + ); + + let content = fs.read_file(dot_path).await.unwrap(); + assert_eq!(content, b"content"); + + // stat with ./ prefix + let meta = fs.stat(dot_path).await; + assert!(meta.is_ok(), "stat with ./ should work"); + + // write via ./ prefix + fs.write_file(Path::new("/tmp/dir/./new.txt"), b"new") + .await + .unwrap(); + let content = fs.read_file(Path::new("/tmp/dir/new.txt")).await.unwrap(); + assert_eq!(content, b"new"); + } + + #[tokio::test] + async fn test_posix_normalize_preserves_semantics() { + // Verify normalization doesn't break parent-exists checks + let fs = InMemoryFs::new(); + + // /tmp exists, /tmp/nonexistent does not + let result = fs + .write_file(Path::new("/tmp/nonexistent/./file.txt"), b"content") + .await; + assert!(result.is_err(), "should fail when parent doesn't exist"); + } } diff --git a/crates/bashkit/src/interpreter/glob.rs b/crates/bashkit/src/interpreter/glob.rs index 5ed7b280..b64cf642 100644 --- a/crates/bashkit/src/interpreter/glob.rs +++ b/crates/bashkit/src/interpreter/glob.rs @@ -616,7 +616,7 @@ impl Interpreter { if p.as_os_str().is_empty() { (self.cwd.clone(), name) } else { - (self.cwd.join(p), name) + (crate::fs::normalize_path(&self.cwd.join(p)), name) } } else { (self.cwd.clone(), name) diff --git a/crates/bashkit/src/interpreter/mod.rs b/crates/bashkit/src/interpreter/mod.rs index 2b90bca4..cc925cbc 100644 --- a/crates/bashkit/src/interpreter/mod.rs +++ b/crates/bashkit/src/interpreter/mod.rs @@ -2033,11 +2033,12 @@ impl Interpreter { // Unary operators let resolve = |p: &str| -> std::path::PathBuf { let path = std::path::Path::new(p); - if path.is_absolute() { + let joined = if path.is_absolute() { path.to_path_buf() } else { self.cwd.join(path) - } + }; + crate::fs::normalize_path(&joined) }; match args[0].as_str() { "-z" => args[1].is_empty(), @@ -6185,14 +6186,15 @@ impl Interpreter { Ok(result) } - /// Resolve a path relative to cwd + /// Resolve a path relative to cwd, normalizing `.` and `..` components. fn resolve_path(&self, path: &str) -> PathBuf { let p = Path::new(path); - if p.is_absolute() { + let joined = if p.is_absolute() { p.to_path_buf() } else { self.cwd.join(p) - } + }; + crate::fs::normalize_path(&joined) } /// Expand an array access expression (`${arr[index]}`). diff --git a/crates/bashkit/src/lib.rs b/crates/bashkit/src/lib.rs index b34b3f88..51d2a9f7 100644 --- a/crates/bashkit/src/lib.rs +++ b/crates/bashkit/src/lib.rs @@ -5439,4 +5439,127 @@ echo missing fi"#, *stderr_chunks ); } + + #[tokio::test] + async fn test_dot_slash_prefix_ls() { + // Issue #1114: ./filename should resolve identically to filename + let mut bash = Bash::new(); + bash.exec("mkdir -p /tmp/blogtest && cd /tmp/blogtest && echo hello > tag_hello.html") + .await + .unwrap(); + + // ls without ./ prefix should work + let result = bash + .exec("cd /tmp/blogtest && ls tag_hello.html") + .await + .unwrap(); + assert_eq!( + result.exit_code, 0, + "ls tag_hello.html should succeed: {}", + result.stderr + ); + assert!(result.stdout.contains("tag_hello.html")); + + // ls with ./ prefix should also work + let result = bash + .exec("cd /tmp/blogtest && ls ./tag_hello.html") + .await + .unwrap(); + assert_eq!( + result.exit_code, 0, + "ls ./tag_hello.html should succeed: {}", + result.stderr + ); + assert!(result.stdout.contains("tag_hello.html")); + } + + #[tokio::test] + async fn test_dot_slash_prefix_glob() { + // Issue #1114: ./*.html should resolve identically to *.html + let mut bash = Bash::new(); + bash.exec("mkdir -p /tmp/globtest && cd /tmp/globtest && echo hello > tag_hello.html") + .await + .unwrap(); + + // glob without ./ prefix + let result = bash.exec("cd /tmp/globtest && echo *.html").await.unwrap(); + assert_eq!( + result.exit_code, 0, + "echo *.html should succeed: {}", + result.stderr + ); + assert!(result.stdout.contains("tag_hello.html")); + + // glob with ./ prefix + let result = bash + .exec("cd /tmp/globtest && echo ./*.html") + .await + .unwrap(); + assert_eq!( + result.exit_code, 0, + "echo ./*.html should succeed: {}", + result.stderr + ); + assert!(result.stdout.contains("tag_hello.html")); + } + + #[tokio::test] + async fn test_dot_slash_prefix_cat() { + // Issue #1114: cat ./filename should work + let mut bash = Bash::new(); + bash.exec("mkdir -p /tmp/cattest && cd /tmp/cattest && echo content123 > myfile.txt") + .await + .unwrap(); + + let result = bash + .exec("cd /tmp/cattest && cat ./myfile.txt") + .await + .unwrap(); + assert_eq!( + result.exit_code, 0, + "cat ./myfile.txt should succeed: {}", + result.stderr + ); + assert!(result.stdout.contains("content123")); + } + + #[tokio::test] + async fn test_dot_slash_prefix_redirect() { + // Issue #1114: redirecting to ./filename should work + let mut bash = Bash::new(); + bash.exec("mkdir -p /tmp/redirtest && cd /tmp/redirtest") + .await + .unwrap(); + + let result = bash + .exec("cd /tmp/redirtest && echo hello > ./output.txt && cat ./output.txt") + .await + .unwrap(); + assert_eq!( + result.exit_code, 0, + "redirect to ./output.txt should succeed: {}", + result.stderr + ); + assert!(result.stdout.contains("hello")); + } + + #[tokio::test] + async fn test_dot_slash_prefix_test_builtin() { + // Issue #1114: test -f ./filename should work + let mut bash = Bash::new(); + bash.exec("mkdir -p /tmp/testbuiltin && cd /tmp/testbuiltin && echo x > myfile.txt") + .await + .unwrap(); + + let result = bash + .exec("cd /tmp/testbuiltin && test -f ./myfile.txt && echo yes") + .await + .unwrap(); + assert_eq!( + result.exit_code, 0, + "test -f ./myfile.txt should succeed: {}", + result.stderr + ); + assert!(result.stdout.contains("yes")); + } }