Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
118 changes: 92 additions & 26 deletions src/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use regex::Regex;
use serde::{Deserialize, Serialize};
use std::collections::{HashMap, HashSet};
use std::fmt::Write as _;
use std::path::{Path, PathBuf};
use std::path::{Component, Path, PathBuf};
use std::str;
use std::str::FromStr;
use tracing::debug;
Expand Down Expand Up @@ -153,21 +153,36 @@ fn normalize_license_file_path(base_dir: &Path, absolute_license_path: &Path) ->
Ok(relative.to_path_buf())
}

fn validate_safe_relative_license_path(path: &Path, source: &str) -> Result<()> {
if path.components().any(|c| {
matches!(
c,
std::path::Component::ParentDir
| std::path::Component::Prefix(_)
| std::path::Component::RootDir
)
}) {
fn resolve_pyproject_metadata_file(
pyproject_dir: &Path,
allowed_metadata_root: &Path,
field: &str,
path: &Path,
) -> Result<PathBuf> {
if path
.components()
.any(|c| matches!(c, Component::Prefix(_) | Component::RootDir))
{
bail!(
"{source} must be a safe relative path inside the project, got `{}`",
"`{field}` path `{}` must be relative to pyproject.toml",
path.display()
);
}
Ok(())
let source = pyproject_dir.join(path);
if path.components().any(|c| matches!(c, Component::ParentDir)) {
let source = source.normalize()?.into_path_buf();
let allowed_metadata_root = allowed_metadata_root.normalize()?.into_path_buf();
if !source.starts_with(&allowed_metadata_root) {
bail!(
"`{field}` path `{}` resolves outside allowed metadata root `{}`",
path.display(),
allowed_metadata_root.display()
);
}
Ok(source)
} else {
Ok(source)
}
}

impl Metadata24 {
Expand All @@ -179,7 +194,21 @@ impl Metadata24 {
pyproject_dir: impl AsRef<Path>,
pyproject_toml: &PyProjectToml,
) -> Result<()> {
let pyproject_dir = pyproject_dir.as_ref();
self.merge_pyproject_toml_with_metadata_root(
pyproject_dir.as_ref(),
pyproject_dir.as_ref(),
pyproject_toml,
)
}

/// Merge pyproject metadata while allowing parent-relative metadata files
/// under `allowed_metadata_root`.
pub(crate) fn merge_pyproject_toml_with_metadata_root(
&mut self,
pyproject_dir: &Path,
allowed_metadata_root: &Path,
pyproject_toml: &PyProjectToml,
) -> Result<()> {
if let Some(project) = &pyproject_toml.project {
let dynamic: HashSet<&str> = project
.dynamic
Expand All @@ -194,8 +223,8 @@ impl Metadata24 {
self.name.clone_from(&project.name);

self.merge_version(pyproject_toml, project)?;
self.merge_readme(pyproject_dir, project)?;
self.merge_license(pyproject_dir, project)?;
self.merge_readme(pyproject_dir, allowed_metadata_root, project)?;
self.merge_license(pyproject_dir, allowed_metadata_root, project)?;
self.merge_people(project);

if let Some(description) = &project.description {
Expand Down Expand Up @@ -283,11 +312,17 @@ impl Metadata24 {
fn merge_readme(
&mut self,
pyproject_dir: &Path,
allowed_metadata_root: &Path,
project: &pyproject_toml::Project,
) -> Result<()> {
match &project.readme {
Some(pyproject_toml::ReadMe::RelativePath(readme_path)) => {
let readme_path = pyproject_dir.join(readme_path);
let readme_path = resolve_pyproject_metadata_file(
pyproject_dir,
allowed_metadata_root,
"project.readme",
Path::new(readme_path),
)?;
let description = Some(fs::read_to_string(&readme_path).context(format!(
"Failed to read readme specified in pyproject.toml, which should be at {}",
readme_path.display()
Expand All @@ -306,7 +341,12 @@ impl Metadata24 {
);
}
if let Some(readme_path) = file {
let readme_path = pyproject_dir.join(readme_path);
let readme_path = resolve_pyproject_metadata_file(
pyproject_dir,
allowed_metadata_root,
"project.readme.file",
Path::new(readme_path),
)?;
let description = Some(fs::read_to_string(&readme_path).context(format!(
"Failed to read readme specified in pyproject.toml, which should be at {}",
readme_path.display()
Expand All @@ -327,6 +367,7 @@ impl Metadata24 {
fn merge_license(
&mut self,
pyproject_dir: &Path,
allowed_metadata_root: &Path,
project: &pyproject_toml::Project,
) -> Result<()> {
if let Some(license) = &project.license {
Expand All @@ -335,11 +376,34 @@ impl Metadata24 {
License::Spdx(license_expr) => self.license_expression = Some(license_expr.clone()),
// Deprecated by PEP 639
License::File { file } => {
let file = file.to_path_buf();
validate_safe_relative_license_path(&file, "`project.license.file`")?;
self.license_file_sources.remove(&file);
if !self.license_files.contains(&file) {
self.license_files.push(file);
let source = resolve_pyproject_metadata_file(
pyproject_dir,
allowed_metadata_root,
"project.license.file",
file,
)?;
let target = if file.components().any(|c| matches!(c, Component::ParentDir)) {
let allowed_metadata_root =
allowed_metadata_root.normalize()?.into_path_buf();
source
.strip_prefix(&allowed_metadata_root)
.with_context(|| {
format!(
"`project.license.file` path `{}` resolves outside allowed metadata root `{}`",
file.display(),
allowed_metadata_root.display()
)
})?
.to_path_buf()
} else {
normalize_license_file_path(pyproject_dir, &source)?
};
self.license_file_sources.remove(&target);
if target != *file {
self.license_file_sources.insert(target.clone(), source);
}
if !self.license_files.contains(&target) {
self.license_files.push(target);
}
Comment thread
messense marked this conversation as resolved.
}
License::Text { text } => self.license = Some(text.clone()),
Expand Down Expand Up @@ -1527,7 +1591,10 @@ A test project
#[test]
fn test_reject_unsafe_project_license_file_path() {
let temp_dir = TempDir::new().unwrap();
let crate_dir = temp_dir.path();
let workspace_root = temp_dir.path().join("workspace");
let crate_dir = workspace_root.join("crate");
fs::create_dir_all(&crate_dir).unwrap();
fs::write(temp_dir.path().join("LICENSE"), "secret license").unwrap();

let pyproject_toml_content = indoc!(
r#"
Expand All @@ -1538,7 +1605,7 @@ A test project
[project]
name = "my-crate"
version = "0.1.0"
license = { file = "../LICENSE" }
license = { file = "../../LICENSE" }
"#
);
fs::write(crate_dir.join("pyproject.toml"), pyproject_toml_content).unwrap();
Expand All @@ -1550,8 +1617,7 @@ A test project
.merge_pyproject_toml(crate_dir, &pyproject_toml)
.unwrap_err();
assert!(
err.to_string()
.contains("`project.license.file` must be a safe relative path"),
err.to_string().contains("outside allowed metadata root"),
"unexpected error: {err:#}"
);
}
Expand Down
12 changes: 11 additions & 1 deletion src/project_layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,17 @@ impl ProjectResolver {
.context("Failed to parse Cargo.toml into python metadata")?;
if let Some(pyproject) = pyproject {
let pyproject_dir = pyproject_file.parent().unwrap();
metadata24.merge_pyproject_toml(pyproject_dir, pyproject)?;
let workspace_root = cargo_metadata.workspace_root.as_std_path();
let allowed_metadata_root = if pyproject_dir.starts_with(workspace_root) {
workspace_root
} else {
pyproject_dir
};
metadata24.merge_pyproject_toml_with_metadata_root(
pyproject_dir,
allowed_metadata_root,
pyproject,
)?;
}

let crate_name = &cargo_toml.package.name;
Expand Down
40 changes: 32 additions & 8 deletions src/source_distribution/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,10 @@ use self::cargo_toml_rewrite::{
rewrite_cargo_toml_targets, strip_non_workspace_tables,
};
pub use self::path_deps::{PathDependency, find_path_deps};
use self::pyproject::{add_pyproject_metadata, add_pyproject_toml, add_python_sources};
use self::pyproject::{
add_pyproject_metadata, add_pyproject_metadata_files, add_pyproject_toml, add_python_sources,
reject_parent_relative_metadata_paths,
};
pub use self::unpack::{UnpackedSdist, unpack_sdist};
use self::utils::{common_path_prefix, is_compiled_artifact, normalize_path};

Expand Down Expand Up @@ -831,11 +834,21 @@ fn add_workspace_manifest(
/// 6. Python sources
fn add_cargo_package_files_to_sdist(
project: &crate::ProjectContext,
pyproject: &PyProjectToml,
pyproject_toml_path: &Path,
writer: &mut VirtualWriter<SDistWriter>,
root_dir: &Path,
) -> Result<()> {
let ctx = SdistContext::new(project, pyproject_toml_path, root_dir)?;
let workspace_root = project.cargo_metadata.workspace_root.as_std_path();
let normalized_pyproject_dir = ctx.pyproject_dir.normalize()?.into_path_buf();
let normalized_workspace_root = workspace_root.normalize()?.into_path_buf();
let allowed_metadata_root = if normalized_pyproject_dir.starts_with(&normalized_workspace_root)
{
workspace_root
} else {
&ctx.pyproject_dir
};

// 1. Add local path dependencies
for (name, path_dep) in ctx.known_path_deps.iter() {
Expand All @@ -851,10 +864,14 @@ fn add_cargo_package_files_to_sdist(
// 4. Add workspace Cargo.toml (if applicable)
add_workspace_manifest(writer, &ctx)?;

// 5. Add pyproject.toml
add_pyproject_toml(writer, &ctx, pyproject_toml_path)?;
// 5. Add pyproject.toml metadata files and collect path rewrites.
let metadata_rewrites =
add_pyproject_metadata_files(writer, pyproject, &ctx, allowed_metadata_root)?;

// 6. Add pyproject.toml
add_pyproject_toml(writer, &ctx, pyproject_toml_path, &metadata_rewrites)?;

// 6. Add python source files
// 7. Add python source files
add_python_sources(writer, &ctx)?;

Ok(())
Expand Down Expand Up @@ -903,16 +920,23 @@ pub fn source_distribution(
&metadata24.get_version_escaped()
));

let pyproject_dir = pyproject_toml_path.parent().unwrap();
match pyproject.sdist_generator() {
SdistGenerator::Cargo => {
add_cargo_package_files_to_sdist(project, &pyproject_toml_path, &mut writer, &root_dir)?
add_cargo_package_files_to_sdist(
project,
pyproject,
&pyproject_toml_path,
&mut writer,
&root_dir,
)?;
}
SdistGenerator::Git => {
add_git_tracked_files_to_sdist(&pyproject_toml_path, &mut writer, &root_dir)?
reject_parent_relative_metadata_paths(pyproject)?;
add_git_tracked_files_to_sdist(&pyproject_toml_path, &mut writer, &root_dir)?;
}
}
};

let pyproject_dir = pyproject_toml_path.parent().unwrap();
add_pyproject_metadata(
&mut writer,
pyproject,
Expand Down
Loading
Loading