diff --git a/crates/kernel/src/path.rs b/crates/kernel/src/path.rs index 7f7adfdd1..27eccdf63 100644 --- a/crates/kernel/src/path.rs +++ b/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 { + 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 { +pub fn clean_path(path: &[u8]) -> Vec { + 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 { 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 { + 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 { + 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"/"); } } diff --git a/crates/kernel/src/syscalls.rs b/crates/kernel/src/syscalls.rs index 0a727d567..ced896430 100644 --- a/crates/kernel/src/syscalls.rs +++ b/crates/kernel/src/syscalls.rs @@ -1672,12 +1672,25 @@ fn parent_path(path: &[u8]) -> Vec { 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,6 +1727,9 @@ 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)?; @@ -1714,6 +1737,9 @@ fn check_parent_writable(proc: &Process, host: &mut dyn HostIO, path: &[u8]) -> } 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);