Avoid calling normalize_path with relative paths that extend beyond…

… the current directory (#3013)

## Summary

It turns out that `normalize_path` (sourced from Cargo) has a subtle
bug. If you pass it a relative path that traverses beyond the root, it
silently drops components. So, e.g., passing `../foo/bar`, it will just
drop the leading `..` and return `foo/bar`.

This PR encodes that behavior as a `Result` and avoids using it in such
cases.

Closes #3012.

Cargo.toml

@@ -99,6 +99,7 @@ miette = { version = "7.2.0" }
 nanoid = { version = "0.4.0" }
 once_cell = { version = "1.19.0" }
 owo-colors = { version = "4.0.0" }
+path-absolutize = { version = "3.1.1" }
 pathdiff = { version = "0.2.1" }
 petgraph = { version = "0.6.4" }
 platform-info = { version = "2.0.2" }

crates/pep508-rs/src/verbatim_url.rs

@@ -36,11 +36,13 @@ impl VerbatimUrl {
     }
 
     /// Create a [`VerbatimUrl`] from a file path.
+    ///
+    /// Assumes that the path is absolute.
     pub fn from_path(path: impl AsRef<Path>) -> Self {
         let path = path.as_ref();
 
         // Normalize the path.
-        let path = normalize_path(path);
+        let path = normalize_path(path).expect("path is absolute");
 
         // Extract the fragment, if it exists.
         let (path, fragment) = split_fragment(&path);
@@ -76,7 +78,7 @@ impl VerbatimUrl {
         };
 
         // Normalize the path.
-        let path = normalize_path(path);
+        let path = normalize_path(&path).expect("path is absolute");
 
         // Extract the fragment, if it exists.
         let (path, fragment) = split_fragment(&path);
@@ -110,7 +112,9 @@ impl VerbatimUrl {
         };
 
         // Normalize the path.
-        let path = normalize_path(path);
+        let Ok(path) = normalize_path(&path) else {
+            return Err(VerbatimUrlError::RelativePath(path));
+        };
 
         // Extract the fragment, if it exists.
         let (path, fragment) = split_fragment(&path);

crates/uv-fs/Cargo.toml

@@ -22,6 +22,7 @@ fs-err = { workspace = true }
 fs2 = { workspace = true }
 junction = { workspace = true }
 once_cell = { workspace = true }
+path-absolutize = { workspace = true }
 tempfile = { workspace = true }
 tokio = { workspace = true, optional = true }
 tracing = { workspace = true }

crates/uv-fs/src/path.rs

@@ -84,8 +84,21 @@ pub fn normalize_url_path(path: &str) -> Cow<'_, str> {
 /// Normalize a path, removing things like `.` and `..`.
 ///
 /// Source: <https://github.com/rust-lang/cargo/blob/b48c41aedbd69ee3990d62a0e2006edbb506a480/crates/cargo-util/src/paths.rs#L76C1-L109C2>
-pub fn normalize_path(path: impl AsRef<Path>) -> PathBuf {
-    let mut components = path.as_ref().components().peekable();
+///
+/// CAUTION: Assumes that the path is already absolute.
+///
+/// CAUTION: This does not resolve symlinks (unlike
+/// [`std::fs::canonicalize`]). This may cause incorrect or surprising
+/// behavior at times. This should be used carefully. Unfortunately,
+/// [`std::fs::canonicalize`] can be hard to use correctly, since it can often
+/// fail, or on Windows returns annoying device paths.
+///
+/// # Errors
+///
+/// When a relative path is provided with `..` components that extend beyond the base directory.
+/// For example, `./a/../../b` cannot be normalized because it escapes the base directory.
+pub fn normalize_path(path: &Path) -> Result<PathBuf, std::io::Error> {
+    let mut components = path.components().peekable();
     let mut ret = if let Some(c @ Component::Prefix(..)) = components.peek().copied() {
         components.next();
         PathBuf::from(c.as_os_str())
@@ -101,14 +114,29 @@ pub fn normalize_path(path: impl AsRef<Path>) -> PathBuf {
             }
             Component::CurDir => {}
             Component::ParentDir => {
-                ret.pop();
+                if !ret.pop() {
+                    return Err(std::io::Error::new(
+                        std::io::ErrorKind::InvalidInput,
+                        "cannot normalize a relative path beyond the base directory",
+                    ));
+                }
             }
             Component::Normal(c) => {
                 ret.push(c);
             }
         }
     }
-    ret
+    Ok(ret)
+}
+
+/// Convert a path to an absolute path, relative to the current working directory.
+///
+/// Unlike [`std::fs::canonicalize`], this function does not resolve symlinks and does not require
+/// the path to exist.
+pub fn absolutize_path(path: &Path) -> Result<Cow<Path>, std::io::Error> {
+    use path_absolutize::Absolutize;
+
+    path.absolutize_from(&*CWD)
 }
 
 /// Like `fs_err::canonicalize`, but with permissive failures on Windows.
@@ -235,7 +263,7 @@ mod tests {
     use super::*;
 
     #[test]
-    fn normalize() {
+    fn test_normalize_url() {
         if cfg!(windows) {
             assert_eq!(
                 normalize_url_path("/C:/Users/ferris/wheel-0.42.0.tar.gz"),
@@ -272,4 +300,20 @@ mod tests {
             );
         }
     }
+
+    #[test]
+    fn test_normalize_path() {
+        let path = Path::new("/a/b/../c/./d");
+        let normalized = normalize_path(path).unwrap();
+        assert_eq!(normalized, Path::new("/a/c/d"));
+
+        let path = Path::new("/a/../c/./d");
+        let normalized = normalize_path(path).unwrap();
+        assert_eq!(normalized, Path::new("/c/d"));
+
+        // This should be an error.
+        let path = Path::new("/a/../../c/./d");
+        let err = normalize_path(path).unwrap_err();
+        assert_eq!(err.kind(), std::io::ErrorKind::InvalidInput);
+    }
 }

crates/uv-interpreter/src/find_python.rs

@@ -6,7 +6,6 @@ use std::path::PathBuf;
 use tracing::{debug, instrument};
 
 use uv_cache::Cache;
-use uv_fs::normalize_path;
 use uv_toolchain::PythonVersion;
 
 use crate::interpreter::InterpreterInfoError;
@@ -52,8 +51,7 @@ pub fn find_requested_python(request: &str, cache: &Cache) -> Result<Option<Inte
         Interpreter::query(executable, cache).map(Some)
     } else {
         // `-p /home/ferris/.local/bin/python3.10`
-        let executable = normalize_path(request);
-
+        let executable = uv_fs::absolutize_path(request.as_ref())?;
         Interpreter::query(executable, cache).map(Some)
     }
 }

crates/uv-interpreter/src/interpreter.rs

@@ -488,12 +488,10 @@ impl InterpreterInfo {
     /// unless the Python executable changes, so we use the executable's last modified
     /// time as a cache key.
     pub(crate) fn query_cached(executable: &Path, cache: &Cache) -> Result<Self, Error> {
-        let executable_bytes = executable.as_os_str().as_encoded_bytes();
-
         let cache_entry = cache.entry(
             CacheBucket::Interpreter,
             "",
-            format!("{}.msgpack", digest(&executable_bytes)),
+            format!("{}.msgpack", digest(&executable)),
         );
 
         let modified = Timestamp::from_path(uv_fs::canonicalize_executable(executable)?)?;

req.txt

@@ -0,0 +1 @@
+httpx @ git+https://github.com/encode/httpx@master