Skip to content

Commit be2503f

Browse files
committed
Allow ancestor traversal when loading package manifest outside of roots
As a special-case; unlike for configs and workspaces, this allows Cargo to search all the way up to the root of the filesystem when loading a package manifest, IFF Cargo is running outside of all configured root directories. This exception doesn't apply if Cargo is running within a configured root directory. This is a trade off between safety and convenience (+ backwards compatibility) that ensures it is possible to unpack a package outside of your home directory (such as `/tmp/my-package`) and then start a build from a subdirectory like `/tmp/my-package/sub/dir` such that Cargo will traverse parent directories looking for `/tmp/my-package/Cargo.toml` but it will not try and load untrusted configs under `/tmp/.cargo` or workspace manifests under `/tmp/Cargo.toml` that could have been created by another user. FIXME: If the first manifest loaded is in fact a workspace manifest we need to make sure we add that as a root directory so that when nested packages within the workspace are built they are able to find their way back to the top of the workspace. Otherwise each nested package build will be limited to the directory of the package.
1 parent 0416ba7 commit be2503f

File tree

3 files changed

+74
-30
lines changed

3 files changed

+74
-30
lines changed

src/cargo/core/workspace.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2024,7 +2024,7 @@ fn find_workspace_root_with_loader(
20242024
gctx: &GlobalContext,
20252025
mut loader: impl FnMut(&Path) -> CargoResult<Option<PathBuf>>,
20262026
) -> CargoResult<Option<PathBuf>> {
2027-
let search_route = gctx.find_manifest_search_route(manifest_path);
2027+
let search_route = gctx.find_workspace_manifest_search_route(manifest_path);
20282028

20292029
// Check if there are any workspace roots that have already been found that would work
20302030
{

src/cargo/util/context/mod.rs

Lines changed: 72 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -617,12 +617,10 @@ impl GlobalContext {
617617
Ok(())
618618
}
619619

620-
/// Find shortest route between `start` and one of the roots in [Self::sorted_roots].
621-
///
622-
/// If no root is found then fallback to root == start so we only search one directory.
623-
/// - This is a safety measure to reduce the risk of reading unsafe state from locations
624-
/// that are not owned by the user (for example building under `/tmp`).
625-
fn find_search_route<P: AsRef<Path>>(&self, start: P) -> CargoResult<SearchRoute> {
620+
fn find_nearest_root<P: AsRef<Path>>(
621+
&self,
622+
start: P,
623+
) -> CargoResult<Option<(PathBuf, PathBuf)>> {
626624
let start = start
627625
.as_ref()
628626
.canonicalize()
@@ -632,31 +630,33 @@ impl GlobalContext {
632630
let root = self
633631
.sorted_roots
634632
.iter()
635-
.filter_map(|root| {
636-
if start.starts_with(root) {
637-
tracing::debug!(
638-
"Found candidate search root {:?} for start {:?}",
639-
root,
640-
start
641-
);
642-
Some(root.clone())
643-
} else {
644-
tracing::debug!(
645-
"Skipping candidate search root {:?} for start {:?}",
646-
root,
647-
start
648-
);
649-
None
650-
}
651-
})
652-
.next();
633+
.find(|root| start.starts_with(root))
634+
.cloned();
635+
636+
if let Some(root) = root {
637+
tracing::debug!("Found candidate root {:?}", root);
638+
Ok(Some((start, root)))
639+
} else {
640+
tracing::debug!("No candidate root found for start {:?}", start);
641+
Ok(None)
642+
}
643+
}
644+
645+
/// Find shortest route between `start` and one of the roots in [Self::sorted_roots].
646+
///
647+
/// If no root is found then fallback to root == start so we only search one directory.
648+
/// - This is a safety measure to reduce the risk of reading unsafe state from locations
649+
/// that are not owned by the user (for example building under `/tmp`).
650+
fn find_search_route<P: AsRef<Path>>(&self, start: P) -> CargoResult<SearchRoute> {
651+
let start = start.as_ref();
652+
let route = self.find_nearest_root(&start)?;
653653

654654
// Cargo no longer allows searching up to the root of the filesystem unless the root is
655655
// explicitly added to `sorted_roots`.
656-
let root = root.unwrap_or_else(|| start.clone());
656+
let route = route.unwrap_or_else(|| (start.to_owned(), start.to_owned()));
657657
Ok(SearchRoute {
658-
start,
659-
root: Some(root),
658+
start: route.0,
659+
root: Some(route.1),
660660
})
661661
}
662662

@@ -711,7 +711,51 @@ impl GlobalContext {
711711
config_search_route
712712
}
713713

714-
pub fn find_manifest_search_route<P: AsRef<Path>>(&self, start: P) -> SearchRoute {
714+
pub fn find_package_manifest_search_route<P: AsRef<Path>>(&self, start: P) -> SearchRoute {
715+
tracing::trace!(
716+
"Finding package manifest search route starting at {:?}",
717+
start.as_ref()
718+
);
719+
720+
let start = start
721+
.as_ref()
722+
.canonicalize()
723+
.expect("failed to canonicalize path");
724+
// Return the existing route if the start == path.
725+
if let Some(route) = &*self.manifest_search_route.borrow() {
726+
if route.start == start {
727+
return route.clone();
728+
}
729+
}
730+
731+
// As a special case, we allow the traversal of parent directories, when
732+
// outside of all root directories to find the package manifest.
733+
//
734+
// This is a trade off between safety and convenience, so it's e.g.
735+
// possible to unpack a package under `/tmp` and start a build from
736+
// `/tmp/my-package/sub/dir` and find `/tmp/my-package/Cargo.toml`, but
737+
// not allow a potentially unsafe `/tmp/Cargo.toml` workspace to be loaded.
738+
let manifest_search_route =
739+
if let Some((start, root)) = self.find_nearest_root(&start).ok().flatten() {
740+
SearchRoute {
741+
start,
742+
root: Some(root),
743+
}
744+
} else {
745+
SearchRoute {
746+
start,
747+
root: None, // Allow searching up to the root of the filesystem.
748+
}
749+
};
750+
*self.manifest_search_route.borrow_mut() = Some(manifest_search_route.clone());
751+
manifest_search_route.clone()
752+
}
753+
754+
pub fn find_workspace_manifest_search_route<P: AsRef<Path>>(&self, start: P) -> SearchRoute {
755+
tracing::trace!(
756+
"Finding workspace manifest search route starting at {:?}",
757+
start.as_ref()
758+
);
715759
let start = start
716760
.as_ref()
717761
.canonicalize()

src/cargo/util/important_paths.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ pub fn find_root_manifest_for_wd(gctx: &GlobalContext, cwd: &Path) -> CargoResul
1010
let invalid_cargo_toml_file_name = "cargo.toml";
1111
let mut invalid_cargo_toml_path_exists = false;
1212

13-
let search_route = gctx.find_manifest_search_route(cwd);
13+
let search_route = gctx.find_package_manifest_search_route(cwd);
1414
for current in paths::ancestors(&search_route.start, search_route.root.as_deref()) {
1515
let manifest = current.join(valid_cargo_toml_file_name);
1616
if manifest.exists() {

0 commit comments

Comments
 (0)