Resolve separate roots for config and manifest searches

It's no longer assumed that we can resolve a single stop_at_root path
for both finding config files and manifests.

GlobalContext now maintains a sorted list of user-configured roots and
caches two resolved SearchRoutes; one for loading config files, the
other for manifests (both automatically invalidated if roots are added
or start paths change).

A `SearchRoute` represents a starting directory and the closest
root directory, where parent traversal will stop.

If searching starts from outside of all configured roots then no
ancestor traversal is allowed.

 - This is a safety measure to reduce the risk of loading unsafe config
   files that aren't owned by the user (e.g. under `c:/.cargo` on
   Windows or `/tmp` on Linux)

By default the user's home directory is added as a root if CARGO_ROOTS
is not set.

 - Combined with the rule above, this means that Cargo has safe defaults
   and will not load configs outside of your home directory, unless you
   explicitly set CARGO_ROOTS.

 - This is also a measure to avoid triggering home directory
   automounters

TODO: there still needs to be a special case for loading a project
manifest, before searching for a workspace. Cargo should be allowed to
walk ancestors outside of all configured roots, when loading the first
Cargo.toml. In this case the directory of the manifest should
immediately be set as a root. This would be a safety / ergonomics
trade-off to allow building of crates under `/tmp/package/nested/dir`
but still disallow attempts to load `/tmp/Cargo.toml` as a workspace.

TODO: Cargo should log a warning when searching for files outside of
any root, so it's easy to see why ancestor config files or manifests
are not being loaded.

Author: rib

benches/benchsuite/benches/global_cache_tracker.rs

@@ -35,7 +35,7 @@ fn initialize_context() -> GlobalContext {
     let cwd = homedir.clone();
     let mut gctx = GlobalContext::new(shell, cwd, homedir);
     gctx.nightly_features_allowed = true;
-    gctx.set_search_stop_path(root());
+    gctx.set_config_search_root(root());
     gctx.configure(
         0,
         false,

crates/cargo-util/src/paths.rs

@@ -441,14 +441,14 @@ pub struct PathAncestors<'a> {
 
 impl<'a> PathAncestors<'a> {
     fn new(path: &'a Path, stop_root_at: Option<&Path>) -> PathAncestors<'a> {
-        let stop_at = env::var("__CARGO_TEST_ROOT")
-            .ok()
-            .map(PathBuf::from)
-            .or_else(|| stop_root_at.map(|p| p.to_path_buf()));
+        tracing::trace!(
+            "creating path ancestors iterator from `{}` to `{}`",
+            path.display(),
+            stop_root_at.map_or("<root>".to_string(), |p| p.display().to_string())
+        );
         PathAncestors {
             current: Some(path),
-            //HACK: avoid reading `~/.cargo/config` when testing Cargo itself.
-            stop_at,
+            stop_at: stop_root_at.map(|p| p.to_path_buf()),
         }
     }
 }
@@ -466,6 +466,7 @@ impl<'a> Iterator for PathAncestors<'a> {
                 }
             }
 
+            tracing::trace!("next path ancestor: `{}`", path.display());
             Some(path)
         } else {
             None

src/bin/cargo/cli.rs

@@ -18,35 +18,94 @@ use crate::util::is_rustup;
 use cargo::core::shell::ColorChoice;
 use cargo::util::style;
 
-fn closest_valid_root<'a>(
-    cwd: &std::path::Path,
-    roots: &[&'a std::path::Path],
-) -> anyhow::Result<Option<&'a std::path::Path>> {
-    let cwd = cwd
-        .canonicalize()
-        .context("could not canonicalize current working directory")?;
-
-    // Assumes that all roots are canonicalized.
-    let ancestor_roots =
-        roots
-            .iter()
-            .filter_map(|r| if cwd.starts_with(r) { Some(*r) } else { None });
-
-    Ok(ancestor_roots.into_iter().max_by_key(|root| {
-        // Prefer the root that is closest to the current working directory.
-        // This is done by counting the number of components in the path.
-        root.components().count()
-    }))
-}
-
 #[tracing::instrument(skip_all)]
 pub fn main(gctx: &mut GlobalContext) -> CliResult {
     // CAUTION: Be careful with using `config` until it is configured below.
     // In general, try to avoid loading config values unless necessary (like
     // the [alias] table).
 
+    // Register root directories.
+    //
+    // A root directory limits how far Cargo can search for files.
+    //
+    // Internally there are two notable roots we need to resolve:
+    //   1. The root when searching for config files (starting directory = cwd).
+    //   2. The root when searching for manifests (starting directory = manifest-path or cwd directory)
+    //
+    // Root directories can be specified via CARGO_ROOTS, --root or the existence of `.cargo/root` files.
+    //
+    // If CARGO_ROOTS is not set, the user's home directory is used as a default root.
+    // - This is a safety measure to avoid reading unsafe config files outside of the user's home
+    //   directory.
+    // - This is also a measure to avoid triggering home directory automounter issues on some
+    //   systems.
+    //
+    // The roots are deduplicated and sorted by their length so we can quickly find the closest root
+    // to a given starting directory (longest ancestor).
+    //
+    // A `SearchRoute` represents a route from a starting directory to the closest root directory.
+    //
+    // When there are no roots above a given starting directory, then a `SearchRoute` will use the
+    // starting directory itself is used as the root.
+    // - This is a safety measure to avoid reading unsafe config files in unknown locations (such as
+    // `/tmp`).
+    //
+    // There are two cached `SearchRoute`s, one for config files and one for workspace manifests,
+    // which are used to avoid repeatedly finding the nearest root directory.
+
+    // Should it be an error if a given root doesn't exist?
+    // A user might configure a root under a `/mnt` directory that is not always mounted?
+
+    let roots_env = gctx.get_env_os("CARGO_ROOTS").map(|s| s.to_owned());
+    if let Some(paths_os) = roots_env {
+        for path in std::env::split_paths(&paths_os) {
+            gctx.add_root(&path)?;
+        }
+    } else {
+        //HACK: avoid reading `~/.cargo/config` when testing Cargo itself.
+        let test_root = gctx.get_env_os("__CARGO_TEST_ROOT").map(|s| s.to_owned());
+        if let Some(test_root) = test_root {
+            tracing::debug!(
+                "no CARGO_ROOTS set, using __CARGO_TEST_ROOT as root: {}",
+                test_root.display()
+            );
+            // This is a hack to avoid reading `~/.cargo/config` when testing Cargo itself.
+            gctx.add_root(&test_root)?;
+        } else if let Some(home) = std::env::home_dir() {
+            tracing::debug!(
+                "no CARGO_ROOTS set, using home directory as root: {}",
+                home.display()
+            );
+            // To be safe by default, and not attempt to read config files outside of the
+            // user's home directory, we implicitly add the home directory as a root.
+            // Ref: https://github.com/rust-lang/rfcs/pull/3279
+            //
+            // This is also a measure to avoid triggering home directory automounter issues
+            gctx.add_root(&home)?;
+        }
+    }
+
     let args = cli(gctx).try_get_matches()?;
 
+    if let Some(cli_roots) = args.get_many::<std::path::PathBuf>("root") {
+        for cli_root in cli_roots {
+            gctx.add_root(cli_root)?;
+        }
+    }
+
+    // Look for any `.cargo/root` markers after we have registered all other roots so
+    // that other roots can stop us from triggering automounter issues.
+    let search_route = gctx.find_config_search_route(gctx.cwd());
+
+    let root_marker = paths::ancestors(&search_route.start, search_route.root.as_deref())
+        .find(|current| current.join(".cargo").join("root").exists());
+    if let Some(marker) = root_marker {
+        tracing::debug!("found .cargo/root marker at {}", marker.display());
+        gctx.add_root(marker)?;
+    } else {
+        tracing::debug!("no .cargo/root marker found");
+    }
+
     // Update the process-level notion of cwd
     if let Some(new_cwd) = args.get_one::<std::path::PathBuf>("directory") {
         // This is a temporary hack.
@@ -70,73 +129,6 @@ pub fn main(gctx: &mut GlobalContext) -> CliResult {
         std::env::set_current_dir(&new_cwd).context("could not change to requested directory")?;
     }
 
-    // A root directories can be specified via CARGO_ROOTS, --root or the existence of a `.cargo/root` files.
-    // If more than one root is specified, the effective root is the one closest to the current working directory.
-    // If CARGO_ROOTS is not set, the user's home directory is used as a default root.
-
-    let cwd = std::env::current_dir().context("could not get current working directory")?;
-    // Windows UNC paths are OK here
-    let cwd = cwd
-        .canonicalize()
-        .context("could not canonicalize current working directory")?;
-
-    // XXX: before looking for `.cargo/root` should we first try and resolve roots
-    // from `CARGO_ROOTS` + --root so we can avoid triggering automounter issues?
-    let root_marker = paths::ancestors(&cwd, gctx.search_stop_path())
-        .find(|current| current.join(".cargo").join("root").exists());
-
-    let mut roots: Vec<std::path::PathBuf> = Vec::new();
-
-    if let Some(root_marker) = root_marker {
-        let pb = root_marker
-            .canonicalize()
-            .context("could not canonicalize .cargo/root")?;
-        roots.push(pb);
-    }
-
-    if let Some(paths_os) = gctx.get_env_os("CARGO_ROOTS") {
-        for path in std::env::split_paths(&paths_os) {
-            let pb = path.canonicalize().context(format!(
-                "could not canonicalize CARGO_ROOTS entry `{}`",
-                path.display()
-            ))?;
-            roots.push(pb);
-        }
-    } else if let Some(home) = std::env::home_dir() {
-        // To be safe by default, and not attempt to read config files outside of the
-        // user's home directory, we implicitly add the home directory as a root.
-        // Ref: https://github.com/rust-lang/rfcs/pull/3279
-        let home = home
-            .canonicalize()
-            .context("could not canonicalize home directory")?;
-        tracing::debug!(
-            "implicitly adding home directory as root: {}",
-            home.display()
-        );
-        roots.push(home);
-    }
-
-    if let Some(cli_roots) = args.get_many::<std::path::PathBuf>("root") {
-        for cli_root in cli_roots {
-            let pb = cli_root
-                .canonicalize()
-                .context("could not canonicalize requested root directory")?;
-            roots.push(pb);
-        }
-    }
-
-    let roots: Vec<_> = roots.iter().map(|p| p.as_path()).collect();
-
-    if let Some(root) = closest_valid_root(&cwd, &roots)? {
-        tracing::debug!("root directory: {}", root.display());
-        gctx.set_search_stop_path(root);
-    } else {
-        tracing::debug!("root limited to cwd: {}", cwd.display());
-        // If we are not running with _any_ root then we are conservative and don't
-        // allow any ancestor traversal.
-        gctx.set_search_stop_path(&cwd);
-    }
-
     // Reload now that we have established the cwd and root
     gctx.reload_cwd()?;
 

src/cargo/core/workspace.rs

@@ -21,7 +21,7 @@ use crate::core::{
 use crate::core::{EitherManifest, Package, SourceId, VirtualManifest};
 use crate::ops;
 use crate::sources::{PathSource, SourceConfigMap, CRATES_IO_INDEX, CRATES_IO_REGISTRY};
-use crate::util::context::FeatureUnification;
+use crate::util::context::{FeatureUnification, SearchRoute};
 use crate::util::edit_distance;
 use crate::util::errors::{CargoResult, ManifestError};
 use crate::util::interning::InternedString;
@@ -746,6 +746,7 @@ impl<'gctx> Workspace<'gctx> {
     /// Returns an error if `manifest_path` isn't actually a valid manifest or
     /// if some other transient error happens.
     fn find_root(&mut self, manifest_path: &Path) -> CargoResult<Option<PathBuf>> {
+        debug!("find_root - {}", manifest_path.display());
         let current = self.packages.load(manifest_path)?;
         match current
             .workspace_config()
@@ -2023,12 +2024,14 @@ fn find_workspace_root_with_loader(
     gctx: &GlobalContext,
     mut loader: impl FnMut(&Path) -> CargoResult<Option<PathBuf>>,
 ) -> CargoResult<Option<PathBuf>> {
+    let search_route = gctx.find_manifest_search_route(manifest_path);
+
     // Check if there are any workspace roots that have already been found that would work
     {
         let roots = gctx.ws_roots.borrow();
         // Iterate through the manifests parent directories until we find a workspace
         // root. Note we skip the first item since that is just the path itself
-        for current in paths::ancestors(manifest_path, gctx.search_stop_path()).skip(1) {
+        for current in paths::ancestors(&search_route.start, search_route.root.as_deref()).skip(1) {
             if let Some(ws_config) = roots.get(current) {
                 if !ws_config.is_excluded(manifest_path) {
                     // Add `Cargo.toml` since ws_root is the root and not the file
@@ -2038,7 +2041,7 @@ fn find_workspace_root_with_loader(
         }
     }
 
-    for ances_manifest_path in find_root_iter(manifest_path, gctx) {
+    for ances_manifest_path in find_root_iter(&search_route, gctx) {
         debug!("find_root - trying {}", ances_manifest_path.display());
         if let Some(ws_root_path) = loader(&ances_manifest_path)? {
             return Ok(Some(ws_root_path));
@@ -2058,10 +2061,10 @@ fn read_root_pointer(member_manifest: &Path, root_link: &str) -> PathBuf {
 }
 
 fn find_root_iter<'a>(
-    manifest_path: &'a Path,
+    search_route: &'a SearchRoute,
     gctx: &'a GlobalContext,
 ) -> impl Iterator<Item = PathBuf> + 'a {
-    LookBehind::new(paths::ancestors(manifest_path, gctx.search_stop_path()).skip(2))
+    LookBehind::new(paths::ancestors(&search_route.start, search_route.root.as_deref()).skip(2))
         .take_while(|path| !path.curr.ends_with("target/package"))
         // Don't walk across `CARGO_HOME` when we're looking for the
         // workspace root. Sometimes a package will be organized with

src/cargo/ops/cargo_new.rs

@@ -802,7 +802,7 @@ fn mk(gctx: &GlobalContext, opts: &MkOptions<'_>) -> CargoResult<()> {
     }
 
     let manifest_path = paths::normalize_path(&path.join("Cargo.toml"));
-    if let Ok(root_manifest_path) = find_root_manifest_for_wd(&manifest_path) {
+    if let Ok(root_manifest_path) = find_root_manifest_for_wd(gctx, &manifest_path) {
         let root_manifest = paths::read(&root_manifest_path)?;
         // Sometimes the root manifest is not a valid manifest, so we only try to parse it if it is.
         // This should not block the creation of the new project. It is only a best effort to

src/cargo/ops/registry/owner.rs

@@ -28,7 +28,7 @@ pub fn modify_owners(gctx: &GlobalContext, opts: &OwnersOptions) -> CargoResult<
     let name = match opts.krate {
         Some(ref name) => name.clone(),
         None => {
-            let manifest_path = find_root_manifest_for_wd(gctx.cwd())?;
+            let manifest_path = find_root_manifest_for_wd(gctx, gctx.cwd())?;
             let ws = Workspace::new(&manifest_path, gctx)?;
             ws.current()?.package_id().name().to_string()
         }

src/cargo/ops/registry/yank.rs

@@ -26,7 +26,7 @@ pub fn yank(
     let name = match krate {
         Some(name) => name,
         None => {
-            let manifest_path = find_root_manifest_for_wd(gctx.cwd())?;
+            let manifest_path = find_root_manifest_for_wd(gctx, gctx.cwd())?;
             let ws = Workspace::new(&manifest_path, gctx)?;
             ws.current()?.package_id().name().to_string()
         }

src/cargo/util/command_prelude.rs

@@ -1077,7 +1077,7 @@ pub fn root_manifest(manifest_path: Option<&Path>, gctx: &GlobalContext) -> Carg
         }
         Ok(path)
     } else {
-        find_root_manifest_for_wd(gctx.cwd())
+        find_root_manifest_for_wd(gctx, gctx.cwd())
     }
 }
 
@@ -1132,7 +1132,7 @@ fn get_profile_candidates() -> Vec<clap_complete::CompletionCandidate> {
 
 fn get_workspace_profile_candidates() -> CargoResult<Vec<clap_complete::CompletionCandidate>> {
     let gctx = new_gctx_for_completions()?;
-    let ws = Workspace::new(&find_root_manifest_for_wd(gctx.cwd())?, &gctx)?;
+    let ws = Workspace::new(&find_root_manifest_for_wd(&gctx, gctx.cwd())?, &gctx)?;
     let profiles = Profiles::new(&ws, InternedString::new("dev"))?;
 
     let mut candidates = Vec::new();
@@ -1216,7 +1216,7 @@ fn get_bin_candidates() -> Vec<clap_complete::CompletionCandidate> {
 fn get_targets_from_metadata() -> CargoResult<Vec<Target>> {
     let cwd = std::env::current_dir()?;
     let gctx = GlobalContext::new(shell::Shell::new(), cwd.clone(), cargo_home_with_cwd(&cwd)?);
-    let ws = Workspace::new(&find_root_manifest_for_wd(&cwd)?, &gctx)?;
+    let ws = Workspace::new(&find_root_manifest_for_wd(&gctx, &cwd)?, &gctx)?;
 
     let packages = ws.members().collect::<Vec<_>>();
 
@@ -1271,7 +1271,10 @@ fn get_target_triples_from_rustup() -> CargoResult<Vec<clap_complete::Completion
 fn get_target_triples_from_rustc() -> CargoResult<Vec<clap_complete::CompletionCandidate>> {
     let cwd = std::env::current_dir()?;
     let gctx = GlobalContext::new(shell::Shell::new(), cwd.clone(), cargo_home_with_cwd(&cwd)?);
-    let ws = Workspace::new(&find_root_manifest_for_wd(&PathBuf::from(&cwd))?, &gctx);
+    let ws = Workspace::new(
+        &find_root_manifest_for_wd(&gctx, &PathBuf::from(&cwd))?,
+        &gctx,
+    );
 
     let rustc = gctx.load_global_rustc(ws.as_ref().ok())?;
 
@@ -1374,7 +1377,7 @@ pub fn get_pkg_id_spec_candidates() -> Vec<clap_complete::CompletionCandidate> {
 fn get_packages() -> CargoResult<Vec<Package>> {
     let gctx = new_gctx_for_completions()?;
 
-    let ws = Workspace::new(&find_root_manifest_for_wd(gctx.cwd())?, &gctx)?;
+    let ws = Workspace::new(&find_root_manifest_for_wd(&gctx, gctx.cwd())?, &gctx)?;
 
     let requested_kinds = CompileKind::from_requested_targets(ws.gctx(), &[])?;
     let mut target_data = RustcTargetData::new(&ws, &requested_kinds)?;