[PHP] fix: canonicalize cwd after chdir

crates/kernel/src/path.rs

@@ -2,32 +2,42 @@ extern crate alloc;
 use alloc::vec::Vec;
 
 /// Resolve a path against a working directory.
-/// If path is absolute (starts with '/'), normalize and return it.
-/// If path is relative, prepend cwd + '/' and normalize.
+/// If path is absolute (starts with '/'), clean and return it.
+/// If path is relative, prepend cwd + '/' and clean.
 pub fn resolve_path(path: &[u8], cwd: &[u8]) -> Vec<u8> {
+    if path.is_empty() {
+        return Vec::new();
+    }
     if path.first() == Some(&b'/') {
-        return normalize_path(path);
+        return clean_path(path);
     }
     let mut resolved = cwd.to_vec();
     if resolved.last() != Some(&b'/') {
         resolved.push(b'/');
     }
     resolved.extend_from_slice(path);
-    normalize_path(&resolved)
+    clean_path(&resolved)
 }
 
-/// Normalize an absolute path by resolving `.` and `..` components.
-/// Removes trailing slashes and redundant separators.
+/// Clean an absolute path without lexically resolving `..`.
+///
+/// POSIX pathname resolution is component-wise: an implementation must look up
+/// an intermediate directory before a later `..` can step back out of it. For
+/// example, `existing/missing/../file` fails with ENOENT because `missing` is
+/// looked up first. Collapsing `missing/..` in this helper would incorrectly
+/// bypass that lookup. Backends perform the real component walk, including
+/// symlink and `..` handling.
+///
+/// This helper only removes redundant separators and `.` components so callers
+/// still pass absolute paths to host backends.
 /// The input path must be absolute (start with '/').
-pub fn normalize_path(path: &[u8]) -> Vec<u8> {
+pub fn clean_path(path: &[u8]) -> Vec<u8> {
+    let has_trailing_slash = path.len() > 1 && path.last() == Some(&b'/');
     let mut components: Vec<&[u8]> = Vec::new();
 
     for component in path.split(|&b| b == b'/') {
         match component {
             b"" | b"." => continue,
-            b".." => {
-                components.pop();
-            }
             _ => {
                 components.push(component);
             }
@@ -43,6 +53,47 @@ pub fn normalize_path(path: &[u8]) -> Vec<u8> {
         result.push(b'/');
         result.extend_from_slice(component);
     }
+    if has_trailing_slash && result.len() > 1 {
+        result.push(b'/');
+    }
+    result
+}
+
+/// Back-compat name for callers that want an absolute path string cleaned for
+/// host I/O. This no longer collapses `..`; see [`clean_path`].
+pub fn normalize_path(path: &[u8]) -> Vec<u8> {
+    clean_path(path)
+}
+
+/// Collapse `.` and `..` components for an already-resolved existing path.
+///
+/// General pathname resolution must not do this before lookup, because
+/// `missing/..` must still fail while resolving `missing`. After `chdir(2)`
+/// has successfully validated the target directory, however, the process cwd
+/// should be stored in canonical form so `getcwd(2)` does not report literal
+/// `.` or `..` components.
+pub fn canonicalize_existing_path(path: &[u8]) -> Vec<u8> {
+    let mut components: Vec<&[u8]> = Vec::new();
+
+    for component in path.split(|&b| b == b'/') {
+        match component {
+            b"" | b"." => {}
+            b".." => {
+                components.pop();
+            }
+            _ => components.push(component),
+        }
+    }
+
+    if components.is_empty() {
+        return alloc::vec![b'/'];
+    }
+
+    let mut result = Vec::new();
+    for component in components {
+        result.push(b'/');
+        result.extend_from_slice(component);
+    }
     result
 }
 
@@ -77,7 +128,7 @@ mod tests {
     #[test]
     fn test_empty_path() {
         let resolved = resolve_path(b"", b"/working/dir");
-        assert_eq!(resolved, b"/working/dir");
+        assert_eq!(resolved, b"");
     }
 
     #[test]
@@ -89,52 +140,66 @@ mod tests {
     #[test]
     fn test_dotdot_relative_path() {
         let resolved = resolve_path(b"../file.txt", b"/working/dir");
-        assert_eq!(resolved, b"/working/file.txt");
+        assert_eq!(resolved, b"/working/dir/../file.txt");
     }
 
     #[test]
     fn test_absolute_path_normalized() {
         let resolved = resolve_path(b"/dev/./pts/../null", b"/working/dir");
-        assert_eq!(resolved, b"/dev/null");
+        assert_eq!(resolved, b"/dev/pts/../null");
+    }
+
+    #[test]
+    fn test_clean_absolute() {
+        assert_eq!(clean_path(b"/a/b/c"), b"/a/b/c");
+    }
+
+    #[test]
+    fn test_clean_dot() {
+        assert_eq!(clean_path(b"/a/./b/./c"), b"/a/b/c");
     }
 
     #[test]
-    fn test_normalize_absolute() {
-        assert_eq!(normalize_path(b"/a/b/c"), b"/a/b/c");
+    fn test_clean_preserves_dotdot() {
+        assert_eq!(clean_path(b"/a/b/../c"), b"/a/b/../c");
     }
 
     #[test]
-    fn test_normalize_dot() {
-        assert_eq!(normalize_path(b"/a/./b/./c"), b"/a/b/c");
+    fn test_clean_preserves_dotdot_past_root() {
+        assert_eq!(clean_path(b"/a/../../b"), b"/a/../../b");
     }
 
     #[test]
-    fn test_normalize_dotdot() {
-        assert_eq!(normalize_path(b"/a/b/../c"), b"/a/c");
+    fn test_clean_root() {
+        assert_eq!(clean_path(b"/"), b"/");
     }
 
     #[test]
-    fn test_normalize_dotdot_past_root() {
-        assert_eq!(normalize_path(b"/a/../../b"), b"/b");
+    fn test_clean_trailing_slash() {
+        assert_eq!(clean_path(b"/a/b/"), b"/a/b/");
     }
 
     #[test]
-    fn test_normalize_root() {
-        assert_eq!(normalize_path(b"/"), b"/");
+    fn test_clean_double_slash() {
+        assert_eq!(clean_path(b"/a//b///c"), b"/a/b/c");
     }
 
     #[test]
-    fn test_normalize_trailing_slash() {
-        assert_eq!(normalize_path(b"/a/b/"), b"/a/b");
+    fn test_clean_only_dotdot() {
+        assert_eq!(clean_path(b"/.."), b"/..");
     }
 
     #[test]
-    fn test_normalize_double_slash() {
-        assert_eq!(normalize_path(b"/a//b///c"), b"/a/b/c");
+    fn test_clean_preserves_trailing_slash_after_dot() {
+        assert_eq!(clean_path(b"/a/./"), b"/a/");
     }
 
     #[test]
-    fn test_normalize_only_dotdot() {
-        assert_eq!(normalize_path(b"/.."), b"/");
+    fn test_canonicalize_existing_path_collapses_dotdot() {
+        assert_eq!(
+            canonicalize_existing_path(b"/a/b/../c/./"),
+            b"/a/c",
+        );
+        assert_eq!(canonicalize_existing_path(b"/.."), b"/");
     }
 }

crates/kernel/src/syscalls.rs

@@ -1672,12 +1672,25 @@ fn parent_path(path: &[u8]) -> Vec<u8> {
     if path == b"/" {
         return alloc::vec![b'/'];
     }
+    let mut end = path.len();
+    while end > 1 && path[end - 1] == b'/' {
+        end -= 1;
+    }
+    let path = &path[..end];
     match path.iter().rposition(|&b| b == b'/') {
         Some(0) | None => alloc::vec![b'/'],
         Some(pos) => path[..pos].to_vec(),
     }
 }
 
+fn trim_trailing_slashes(path: &[u8]) -> &[u8] {
+    let mut end = path.len();
+    while end > 1 && path[end - 1] == b'/' {
+        end -= 1;
+    }
+    &path[..end]
+}
+
 fn check_search_dir(proc: &Process, host: &mut dyn HostIO, path: &[u8]) -> Result<(), Errno> {
     let st = host.host_stat(path)?;
     if st.st_mode & S_IFMT != S_IFDIR {
@@ -1687,6 +1700,13 @@ fn check_search_dir(proc: &Process, host: &mut dyn HostIO, path: &[u8]) -> Resul
 }
 
 fn check_search_path(proc: &Process, host: &mut dyn HostIO, path: &[u8]) -> Result<(), Errno> {
+    if path.is_empty() {
+        return Err(Errno::ENOENT);
+    }
+    let trimmed = trim_trailing_slashes(path);
+    if trimmed.len() != path.len() {
+        return check_search_dir_chain(proc, host, trimmed);
+    }
     let parent = parent_path(path);
     check_search_dir_chain(proc, host, &parent)
 }
@@ -1707,13 +1727,19 @@ fn check_search_dir_chain(proc: &Process, host: &mut dyn HostIO, dir: &[u8]) ->
 }
 
 fn check_parent_writable(proc: &Process, host: &mut dyn HostIO, path: &[u8]) -> Result<(), Errno> {
+    if path.is_empty() {
+        return Err(Errno::ENOENT);
+    }
     let parent = parent_path(path);
     check_search_dir_chain(proc, host, &parent)?;
     let st = host.host_stat(&parent)?;
     check_access(proc, &st, W_OK | X_OK)
 }
 
 fn check_sticky_child(proc: &Process, host: &mut dyn HostIO, path: &[u8]) -> Result<(), Errno> {
+    if path.is_empty() {
+        return Err(Errno::ENOENT);
+    }
     let parent = parent_path(path);
     let parent_st = host.host_stat(&parent)?;
     if parent_st.st_mode & S_ISVTX == 0 || proc.euid == 0 {
@@ -4318,20 +4344,21 @@ pub fn sys_access(
 /// Validates that the path exists and is a directory via host_stat.
 pub fn sys_chdir(proc: &mut Process, host: &mut dyn HostIO, path: &[u8]) -> Result<(), Errno> {
     let resolved = crate::path::resolve_path(path, &proc.cwd);
+    let canonical = || crate::path::canonicalize_existing_path(&resolved);
     // Check virtual filesystems first (procfs, devfs), then fall through to host
     if let Some(entry) = crate::procfs::match_procfs(&resolved, proc.pid) {
         let st = crate::procfs::procfs_stat(&entry, 0, true);
         if st.st_mode & wasm_posix_shared::mode::S_IFMT != wasm_posix_shared::mode::S_IFDIR {
             return Err(Errno::ENOTDIR);
         }
-        proc.cwd = resolved;
+        proc.cwd = canonical();
         return Ok(());
     }
     if let Some(st) = crate::devfs::match_devfs_stat(&resolved, proc.euid, proc.egid) {
         if st.st_mode & wasm_posix_shared::mode::S_IFMT != wasm_posix_shared::mode::S_IFDIR {
             return Err(Errno::ENOTDIR);
         }
-        proc.cwd = resolved;
+        proc.cwd = canonical();
         return Ok(());
     }
     // Validate the path exists and is a directory
@@ -4342,7 +4369,7 @@ pub fn sys_chdir(proc: &mut Process, host: &mut dyn HostIO, path: &[u8]) -> Resu
         return Err(Errno::ENOTDIR);
     }
     check_access(proc, &stat, X_OK)?;
-    proc.cwd = resolved;
+    proc.cwd = canonical();
     Ok(())
 }
 
@@ -4353,7 +4380,7 @@ pub fn sys_fchdir(proc: &mut Process, fd: i32) -> Result<(), Errno> {
     if ofd.file_type != FileType::Directory {
         return Err(Errno::ENOTDIR);
     }
-    proc.cwd = ofd.path.clone();
+    proc.cwd = crate::path::canonicalize_existing_path(&ofd.path);
     Ok(())
 }
 
@@ -10951,6 +10978,33 @@ mod tests {
     /// that call sys_bind/sys_connect for AF_UNIX must hold this lock.
     static UNIX_REGISTRY_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(());
 
+    #[test]
+    fn test_parent_path_ignores_trailing_slash() {
+        assert_eq!(parent_path(b"/tmp/newdir/"), b"/tmp");
+        assert_eq!(parent_path(b"/tmp/newdir///"), b"/tmp");
+        assert_eq!(parent_path(b"/tmp"), b"/");
+        assert_eq!(parent_path(b"/"), b"/");
+    }
+
+    #[test]
+    fn test_stat_trailing_slash_requires_directory() {
+        let mut proc = Process::new(1);
+        let mut host = MockHostIO::new();
+
+        assert_eq!(
+            sys_stat(&mut proc, &mut host, b"/tmp/file/").unwrap_err(),
+            Errno::ENOTDIR,
+        );
+    }
+
+    #[test]
+    fn test_empty_path_is_enoent() {
+        let mut proc = Process::new(1);
+        let mut host = MockHostIO::new();
+
+        assert_eq!(sys_stat(&mut proc, &mut host, b"").unwrap_err(), Errno::ENOENT);
+    }
+
     fn test_path_is_dir(path: &[u8]) -> bool {
         path.ends_with(b"/")
             || path.ends_with(b"dir")
@@ -12111,6 +12165,21 @@ mod tests {
         assert_eq!(&buf[..n], b"/tmp/subdir\0");
     }
 
+    #[test]
+    fn test_chdir_canonicalizes_dotdot_in_cwd() {
+        let mut proc = Process::new(1);
+        let mut host = MockHostIO::new();
+        sys_chdir(&mut proc, &mut host, b"/tmp").unwrap();
+        sys_chdir(&mut proc, &mut host, b"subdir").unwrap();
+        host.set_dir_with_owner(b"/tmp/subdir/..", 0, 0, 0o755);
+
+        sys_chdir(&mut proc, &mut host, b"..").unwrap();
+
+        let mut buf = [0u8; 256];
+        let n = sys_getcwd(&proc, &mut buf).unwrap();
+        assert_eq!(&buf[..n], b"/tmp\0");
+    }
+
     #[test]
     fn test_chdir_rejects_non_directory() {
         let mut proc = Process::new(1);