From 9e7fd601a5f50f7915d9efb48cc8f4b569781bc6 Mon Sep 17 00:00:00 2001 From: Suliman Abdulrazzaq Date: Sun, 9 Aug 2026 15:14:37 +0300 Subject: [PATCH 1/3] fix(runtime): normalize Windows workspace paths Keep extended-length and regular Windows paths comparable when enforcing workspace boundaries, including first-run roots that cannot yet be canonicalized. Add regression coverage for DOS and UNC path forms. Signed-off-by: Suliman Abdulrazzaq --- rust/crates/runtime/src/file_ops.rs | 60 ++++++++++++++++++++++++++--- 1 file changed, 55 insertions(+), 5 deletions(-) diff --git a/rust/crates/runtime/src/file_ops.rs b/rust/crates/runtime/src/file_ops.rs index aa7b58135e..dfd4c463b6 100644 --- a/rust/crates/runtime/src/file_ops.rs +++ b/rust/crates/runtime/src/file_ops.rs @@ -40,7 +40,9 @@ fn is_binary_file(path: &Path) -> io::Result { /// the workspace boundary (e.g. via `../` traversal or symlink). #[allow(dead_code)] fn validate_workspace_boundary(resolved: &Path, workspace_root: &Path) -> io::Result<()> { - if !resolved.starts_with(workspace_root) { + let resolved = normalize_for_comparison(resolved); + let workspace_root = normalize_for_comparison(workspace_root); + if !resolved.starts_with(&workspace_root) { return Err(io::Error::new( io::ErrorKind::PermissionDenied, format!( @@ -527,9 +529,29 @@ fn build_grep_content_output( } fn canonicalize_workspace_root(workspace_root: &Path) -> PathBuf { - workspace_root + let canonical = workspace_root .canonicalize() - .unwrap_or_else(|_| workspace_root.to_path_buf()) + .unwrap_or_else(|_| workspace_root.to_path_buf()); + normalize_for_comparison(&canonical) +} + +/// Normalize Windows' extended-length path prefix before comparing paths. +/// +/// `std::fs::canonicalize` returns paths with a `\\?\` prefix on Windows, +/// while a workspace root may still be represented as a regular path when it +/// cannot be canonicalized (for example, while a workspace is bootstrapping). +/// These paths refer to the same location but do not compare equal with +/// [`Path::starts_with`]. Keep the comparison representation stable without +/// changing the paths returned to callers. +fn normalize_for_comparison(path: &Path) -> PathBuf { + let raw = path.to_string_lossy(); + if let Some(unc_path) = raw.strip_prefix(r"\\?\UNC\") { + return PathBuf::from(format!(r"\\{unc_path}")); + } + if let Some(dos_path) = raw.strip_prefix(r"\\?\") { + return PathBuf::from(dos_path); + } + path.to_path_buf() } fn should_skip_glob_dir(entry: &DirEntry) -> bool { @@ -777,8 +799,9 @@ mod tests { use super::{ component_contains_glob, derive_glob_walk_root, edit_file, expand_braces, glob_search, - grep_search, is_symlink_escape, read_file, read_file_in_workspace, write_file, - write_file_in_workspace, GrepSearchInput, MAX_WRITE_SIZE, + grep_search, is_symlink_escape, normalize_for_comparison, read_file, + read_file_in_workspace, validate_workspace_boundary, write_file, write_file_in_workspace, + GrepSearchInput, MAX_WRITE_SIZE, }; fn temp_path(name: &str) -> std::path::PathBuf { @@ -906,6 +929,33 @@ mod tests { let _ = std::fs::remove_dir_all(&outside); } + #[test] + fn normalizes_windows_extended_length_prefixes_for_boundary_checks() { + let dos_path = PathBuf::from(r"C:\workspace\file.txt"); + let extended_dos_path = PathBuf::from(r"\\?\C:\workspace\file.txt"); + assert_eq!( + normalize_for_comparison(&extended_dos_path), + dos_path, + "extended DOS paths should compare like their regular form" + ); + + let unc_path = PathBuf::from(r"\\server\share\workspace\file.txt"); + let extended_unc_path = PathBuf::from(r"\\?\UNC\server\share\workspace\file.txt"); + assert_eq!( + normalize_for_comparison(&extended_unc_path), + unc_path, + "extended UNC paths should compare like their regular form" + ); + } + + #[test] + fn accepts_equivalent_extended_path_at_workspace_boundary() { + let root = PathBuf::from(r"C:\workspace"); + let resolved = PathBuf::from(r"\\?\C:\workspace\src\main.rs"); + validate_workspace_boundary(&resolved, &root) + .expect("equivalent Windows path representations should be accepted"); + } + #[test] #[cfg(unix)] fn workspace_write_rejects_parent_symlink_escape_regression_3007_class() { From dbf538972987c67a824ae7b26cba16dc2e6bc532 Mon Sep 17 00:00:00 2001 From: Suliman Abdulrazzaq Date: Thu, 17 Sep 2026 13:15:05 +0000 Subject: [PATCH 2/3] test(runtime): gate the Windows extended-path boundary test to Windows accepts_equivalent_extended_path_at_workspace_boundary builds its paths with backslash separators. Backslashes only separate path components on Windows, so on Linux each path is a single component, the boundary check rejects the path, and cargo test fails. Run the test only on Windows, and call validate_workspace_boundary through super:: so non-Windows test builds do not warn about an unused import. --- rust/crates/runtime/src/file_ops.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/rust/crates/runtime/src/file_ops.rs b/rust/crates/runtime/src/file_ops.rs index dfd4c463b6..7ad65c99d1 100644 --- a/rust/crates/runtime/src/file_ops.rs +++ b/rust/crates/runtime/src/file_ops.rs @@ -800,8 +800,8 @@ mod tests { use super::{ component_contains_glob, derive_glob_walk_root, edit_file, expand_braces, glob_search, grep_search, is_symlink_escape, normalize_for_comparison, read_file, - read_file_in_workspace, validate_workspace_boundary, write_file, write_file_in_workspace, - GrepSearchInput, MAX_WRITE_SIZE, + read_file_in_workspace, write_file, write_file_in_workspace, GrepSearchInput, + MAX_WRITE_SIZE, }; fn temp_path(name: &str) -> std::path::PathBuf { @@ -948,11 +948,13 @@ mod tests { ); } + // Backslash-separated paths only parse into path components on Windows. #[test] + #[cfg(windows)] fn accepts_equivalent_extended_path_at_workspace_boundary() { let root = PathBuf::from(r"C:\workspace"); let resolved = PathBuf::from(r"\\?\C:\workspace\src\main.rs"); - validate_workspace_boundary(&resolved, &root) + super::validate_workspace_boundary(&resolved, &root) .expect("equivalent Windows path representations should be accepted"); } From dc346498fbedc9b76e49921f514b7c072bff6479 Mon Sep 17 00:00:00 2001 From: Suliman Abdulrazzaq Date: Fri, 18 Sep 2026 11:58:26 +0300 Subject: [PATCH 3/3] fix(runtime): accept other Windows spellings of a workspace path validate_workspace_boundary compared paths lexically, so a location inside the workspace spelled with different letter case (a working directory typed as c:\users\... for C:\Users\...) or through the \\.\ device namespace was reported as escaping the workspace. When the lexical comparison fails, compare the path's canonical form before reporting an escape. A path whose canonical form is outside the workspace stays rejected. --- rust/crates/runtime/src/file_ops.rs | 86 +++++++++++++++++++++++++---- 1 file changed, 75 insertions(+), 11 deletions(-) diff --git a/rust/crates/runtime/src/file_ops.rs b/rust/crates/runtime/src/file_ops.rs index 7ad65c99d1..7a62447a34 100644 --- a/rust/crates/runtime/src/file_ops.rs +++ b/rust/crates/runtime/src/file_ops.rs @@ -40,19 +40,32 @@ fn is_binary_file(path: &Path) -> io::Result { /// the workspace boundary (e.g. via `../` traversal or symlink). #[allow(dead_code)] fn validate_workspace_boundary(resolved: &Path, workspace_root: &Path) -> io::Result<()> { - let resolved = normalize_for_comparison(resolved); let workspace_root = normalize_for_comparison(workspace_root); - if !resolved.starts_with(&workspace_root) { - return Err(io::Error::new( - io::ErrorKind::PermissionDenied, - format!( - "path {} escapes workspace boundary {}", - resolved.display(), - workspace_root.display() - ), - )); + let comparable = normalize_for_comparison(resolved); + if comparable.starts_with(&workspace_root) || resolves_within(resolved, &workspace_root) { + return Ok(()); } - Ok(()) + Err(io::Error::new( + io::ErrorKind::PermissionDenied, + format!( + "path {} escapes workspace boundary {}", + comparable.display(), + workspace_root.display() + ), + )) +} + +/// Whether `resolved` names a location inside `workspace_root` once the +/// filesystem resolves it. Windows accepts several spellings of one location +/// that [`Path::starts_with`] treats as different: letter case (a working +/// directory typed as `c:\users\...` for `C:\Users\...`), the `\\.\` device +/// namespace, and 8.3 short names. Only consulted after the lexical comparison +/// fails, so it can accept such a spelling but never widens what already +/// compared inside, and a path whose canonical form is outside stays rejected. +fn resolves_within(resolved: &Path, workspace_root: &Path) -> bool { + resolved + .canonicalize() + .is_ok_and(|canonical| normalize_for_comparison(&canonical).starts_with(workspace_root)) } /// Text payload returned by file-reading operations. @@ -958,6 +971,57 @@ mod tests { .expect("equivalent Windows path representations should be accepted"); } + #[test] + #[cfg(unix)] + fn accepts_a_path_that_resolves_inside_the_workspace() { + let workspace = temp_path("boundary-alias-workspace"); + let aliases = temp_path("boundary-alias-links"); + std::fs::create_dir_all(workspace.join("src")).expect("workspace dir should be created"); + std::fs::create_dir_all(&aliases).expect("alias dir should be created"); + let alias = aliases.join("src"); + std::os::unix::fs::symlink(workspace.join("src"), &alias).expect("symlink should create"); + let root = workspace + .canonicalize() + .expect("workspace should canonicalize"); + + super::validate_workspace_boundary(&alias, &root) + .expect("a spelling that resolves inside the workspace should be accepted"); + super::validate_workspace_boundary(&aliases, &root) + .expect_err("a path that resolves outside the workspace must stay rejected"); + + let _ = std::fs::remove_dir_all(&workspace); + let _ = std::fs::remove_dir_all(&aliases); + } + + // Windows resolves one location from spellings that compare unequal as + // paths, letter case and the `\\.\` device namespace among them. + #[test] + #[cfg(windows)] + fn accepts_other_windows_spellings_of_a_workspace_path() { + let workspace = temp_path("Boundary-Case-Workspace"); + std::fs::create_dir_all(workspace.join("src")).expect("workspace dir should be created"); + let root = workspace + .canonicalize() + .expect("workspace should canonicalize"); + + let lower = PathBuf::from(workspace.join("src").to_string_lossy().to_lowercase()); + super::validate_workspace_boundary(&lower, &root) + .expect("a lower-cased spelling of a workspace path should be accepted"); + + let device = PathBuf::from(format!(r"\\.\{}", workspace.join("src").display())); + super::validate_workspace_boundary(&device, &root) + .expect("a device-namespace spelling of a workspace path should be accepted"); + + let parent = workspace + .parent() + .expect("workspace has a parent") + .to_path_buf(); + super::validate_workspace_boundary(&parent, &root) + .expect_err("the workspace's parent must stay rejected"); + + let _ = std::fs::remove_dir_all(&workspace); + } + #[test] #[cfg(unix)] fn workspace_write_rejects_parent_symlink_escape_regression_3007_class() {