diff --git a/pulp_rust/app/tasks/publishing.py b/pulp_rust/app/tasks/publishing.py index 1204a9b..a653d77 100644 --- a/pulp_rust/app/tasks/publishing.py +++ b/pulp_rust/app/tasks/publishing.py @@ -1,5 +1,6 @@ import hashlib import struct +import tarfile from django.db import IntegrityError @@ -8,6 +9,7 @@ from pulp_rust.app.models import RustContent, RustDependency, RustRepository from pulp_rust.app.utils import ( + UnsupportedDependencyError, canonicalize_crate_name, extract_cargo_toml, extract_dependencies, @@ -60,6 +62,21 @@ async def apublish_package(repository_pk, metadata, crate_path): metadata: Parsed JSON metadata from the cargo publish request. crate_path: Filesystem path to the .crate tarball. """ + # Extract authoritative metadata from the Cargo.toml inside the .crate tarball. + # The publish JSON metadata is NOT authoritative - a rogue client can send metadata + # that doesn't match the actual package. We only use the JSON name/vers to locate the + # Cargo.toml within the tarball, then extract everything from the Cargo.toml itself. + # See: https://github.com/rust-lang/cargo/issues/14492 + # https://github.com/rust-lang/crates.io/pull/7238 + try: + with open(crate_path, "rb") as f: + cargo_toml = extract_cargo_toml(f, metadata["name"], metadata["vers"]) + deps = extract_dependencies(cargo_toml) + except UnsupportedDependencyError as e: + return {"cargo_error": str(e)} + except (tarfile.TarError, EOFError, KeyError, FileNotFoundError, ValueError): + return {"cargo_error": "invalid crate file"} + repository = await RustRepository.objects.aget(pk=repository_pk) # Create the artifact from the .crate file, or reuse an existing one @@ -73,14 +90,6 @@ async def apublish_package(repository_pk, metadata, crate_path): except IntegrityError: artifact = await Artifact.objects.aget(sha256=cksum) - # Extract authoritative metadata from the Cargo.toml inside the .crate tarball. - # The publish JSON metadata is NOT authoritative - a rogue client can send metadata - # that doesn't match the actual package. We only use the JSON name/vers to locate the - # Cargo.toml within the tarball, then extract everything from the Cargo.toml itself. - # See: https://github.com/rust-lang/cargo/issues/14492 - # https://github.com/rust-lang/crates.io/pull/7238 - with artifact.file.open("rb") as f: - cargo_toml = extract_cargo_toml(f, metadata["name"], metadata["vers"]) package = cargo_toml.get("package", {}) name = package["name"] @@ -89,9 +98,6 @@ async def apublish_package(repository_pk, metadata, crate_path): # build metadata as identical, and the index must not contain duplicates. vers = strip_semver_build_metadata(package["version"]) - # Build dependency list from the Cargo.toml (authoritative source) - deps = extract_dependencies(cargo_toml) - # Reuse existing content if it already exists in the domain with the same # checksum (e.g. from a pull-through cache or another repository's publish). # Content in Pulp is globally shared - the same object can belong to diff --git a/pulp_rust/app/utils.py b/pulp_rust/app/utils.py index 70a4fc5..fd2df45 100644 --- a/pulp_rust/app/utils.py +++ b/pulp_rust/app/utils.py @@ -37,6 +37,20 @@ def _normalize_req(version_str): return f"^{version_str}" +CRATES_IO_INDEX = "https://github.com/rust-lang/crates.io-index" + + +class UnsupportedDependencyError(ValueError): + """A Cargo.toml dependency that cannot be translated to an index entry.""" + + def __init__(self, name, registry): + self.name = name + self.registry = registry + super().__init__( + f"dependency `{name}` names registry `{registry}` without a `registry-index` URL" + ) + + def parse_dep(name, spec, kind="normal", target=None): """Convert a single Cargo.toml dependency entry to index format.""" if isinstance(spec, str): @@ -49,11 +63,16 @@ def parse_dep(name, spec, kind="normal", target=None): "default_features": True, "target": target, "kind": kind, - "registry": None, + "registry": CRATES_IO_INDEX, "package": None, } # Table form: dep = { version = "1.0", optional = true, ... } + if "registry" in spec and "registry-index" not in spec: + # `cargo package` rewrites a named registry to `registry-index`. A bare name can only + # be resolved with the client's configuration, so treating it as crates.io could + # point at the wrong crate. + raise UnsupportedDependencyError(name, spec["registry"]) dep = { "name": name, "req": _normalize_req(spec.get("version", "*")), @@ -62,7 +81,7 @@ def parse_dep(name, spec, kind="normal", target=None): "default_features": spec.get("default-features", True), "target": target, "kind": kind, - "registry": spec.get("registry"), + "registry": spec.get("registry-index") or CRATES_IO_INDEX, "package": None, } # If the dep was renamed, "name" in the index is the alias (the key), diff --git a/pulp_rust/app/views.py b/pulp_rust/app/views.py index 482039c..985b773 100644 --- a/pulp_rust/app/views.py +++ b/pulp_rust/app/views.py @@ -431,7 +431,9 @@ def put(self, request, **kwargs): "crate_path": tmp.name, }, ) - has_task_completed(task) + result = has_task_completed(task) + if result and (error := result.get("cargo_error")): + return cargo_error(error) finally: os.unlink(tmp.name) @@ -577,7 +579,7 @@ def has_task_completed(task): Verify whether an immediate task ran properly. Returns: - bool: True if the task ended successfully. + dict | None: The result returned by a completed task. Raises: Exception: If an error occured during the task's runtime. @@ -585,8 +587,9 @@ def has_task_completed(task): """ if task.state == "completed": + result = task.result task.delete() - return True + return result elif task.state == "canceled": raise Throttled() else: diff --git a/pulp_rust/tests/functional/api/test_publish.py b/pulp_rust/tests/functional/api/test_publish.py index ac3c0c4..4a245fa 100644 --- a/pulp_rust/tests/functional/api/test_publish.py +++ b/pulp_rust/tests/functional/api/test_publish.py @@ -10,12 +10,15 @@ so keep these fidelity checks passing. """ +import io +import tarfile from urllib.parse import urljoin import pytest from pulp_rust.tests.functional.utils import ( CRATES_IO_URL, + as_alternate_registry_entry, assert_index_entry_matches_upstream, build_publish_metadata, cargo_publish, @@ -61,7 +64,9 @@ def test_cargo_publish_and_index_fidelity( # Fetch the index from Pulp and compare against crates.io pulp_entry = get_index_entry(base, "se/rd/serde", "1.0.210") - assert_index_entry_matches_upstream(pulp_entry, upstream_index_entry) + assert_index_entry_matches_upstream( + pulp_entry, as_alternate_registry_entry(upstream_index_entry) + ) def test_cargo_publish_duplicate_rejected( @@ -142,7 +147,9 @@ def test_cargo_publish_ignores_tampered_json_metadata( # The index entry should match crates.io (i.e. the Cargo.toml), not the tampered JSON pulp_entry = get_index_entry(base, "se/rd/serde", "1.0.210") - assert_index_entry_matches_upstream(pulp_entry, upstream_index_entry) + assert_index_entry_matches_upstream( + pulp_entry, as_alternate_registry_entry(upstream_index_entry) + ) def test_cargo_publish_octet_stream_content_type( @@ -198,6 +205,51 @@ def test_cargo_publish_uploads_disabled( assert any("does not allow uploads" in e["detail"] for e in errors) +def test_cargo_publish_invalid_crate_file_rejected( + rust_repo_factory, + rust_distribution_factory, + cargo_registry_url, + admin_auth_headers, +): + """An invalid crate file should produce Cargo's normal error response.""" + repository = rust_repo_factory() + distribution = rust_distribution_factory(repository=repository.pulp_href, allow_uploads=True) + base = cargo_registry_url(distribution.base_path) + + metadata = {"name": "invalid-crate", "vers": "0.1.0", "deps": [], "features": {}} + response = cargo_publish(base, metadata, b"not a crate", headers=admin_auth_headers) + + assert response.status_code == 400 + assert response.json()["errors"][0]["detail"] == "invalid crate file" + + +def test_cargo_publish_named_registry_dependency_rejected( + rust_repo_factory, + rust_distribution_factory, + cargo_registry_url, + admin_auth_headers, +): + """A dependency naming a registry without its index URL cannot be resolved by Pulp.""" + cargo_toml = ( + b'[package]\nname = "named-dep"\nversion = "0.1.0"\n\n' + b'[dependencies]\nfoo = { version = "1.0", registry = "my-named-registry" }\n' + ) + buf = io.BytesIO() + with tarfile.open(fileobj=buf, mode="w:gz") as tar: + info = tarfile.TarInfo(name="named-dep-0.1.0/Cargo.toml") + info.size = len(cargo_toml) + tar.addfile(info, io.BytesIO(cargo_toml)) + + repository = rust_repo_factory() + distribution = rust_distribution_factory(repository=repository.pulp_href, allow_uploads=True) + base = cargo_registry_url(distribution.base_path) + + metadata = {"name": "named-dep", "vers": "0.1.0", "deps": [], "features": {}} + response = cargo_publish(base, metadata, buf.getvalue(), headers=admin_auth_headers) + assert response.status_code == 400 + assert "my-named-registry" in response.json()["errors"][0]["detail"] + + def test_cargo_publish_invalid_name_rejected( rust_repo_factory, rust_distribution_factory, diff --git a/pulp_rust/tests/functional/utils.py b/pulp_rust/tests/functional/utils.py index 90aa583..b35b386 100644 --- a/pulp_rust/tests/functional/utils.py +++ b/pulp_rust/tests/functional/utils.py @@ -8,7 +8,7 @@ import aiohttp import requests -from pulp_rust.app.utils import extract_cargo_toml, extract_dependencies +from pulp_rust.app.utils import CRATES_IO_INDEX, extract_cargo_toml, extract_dependencies CRATES_IO_URL = "sparse+https://index.crates.io/" CRATES_IO_DOWNLOAD_URL = "https://static.crates.io/crates/" @@ -221,6 +221,19 @@ def download_crate_from_upstream(name, version): # --- Assertion helpers --- +def as_alternate_registry_entry(upstream_entry): + """Return a crates.io index entry as an alternate registry must serve it. + + crates.io omits `registry` on dependencies from itself. Any other registry has to name the + crates.io index explicitly, or Cargo resolves them against that registry instead. + """ + deps = [ + {**dep, "registry": dep.get("registry") or CRATES_IO_INDEX} + for dep in upstream_entry["deps"] + ] + return {**upstream_entry, "deps": deps} + + def assert_index_entry_matches_upstream(pulp_entry, upstream_entry): """Assert that a Pulp-served index entry matches the upstream for all stored fields.""" for field in INDEX_FIELDS_TO_COMPARE: diff --git a/pulp_rust/tests/unit/test_utils.py b/pulp_rust/tests/unit/test_utils.py index 086fbee..3467c33 100644 --- a/pulp_rust/tests/unit/test_utils.py +++ b/pulp_rust/tests/unit/test_utils.py @@ -10,6 +10,8 @@ django.setup() from pulp_rust.app.utils import ( # noqa: E402 + CRATES_IO_INDEX, + UnsupportedDependencyError, canonicalize_crate_name, extract_cargo_toml, extract_dependencies, @@ -44,7 +46,7 @@ def test_simple_string_spec(self): "default_features": True, "target": None, "kind": "normal", - "registry": None, + "registry": CRATES_IO_INDEX, "package": None, } @@ -63,7 +65,7 @@ def test_table_spec_all_fields(self): "features": ["derive", "std"], "optional": True, "default-features": False, - "registry": "https://my-registry.example.com/", + "registry-index": "https://my-registry.example.com/", "package": "serde_real", } result = parse_dep("my_serde", spec) @@ -75,6 +77,29 @@ def test_table_spec_all_fields(self): assert result["registry"] == "https://my-registry.example.com/" assert result["package"] == "serde_real" + def test_string_spec_registry_is_crates_io(self): + assert parse_dep("serde", "1.0")["registry"] == CRATES_IO_INDEX + + def test_table_spec_without_registry_defaults_to_crates_io(self): + result = parse_dep("serde", {"version": "1.0"}) + assert result["registry"] == CRATES_IO_INDEX + + def test_table_spec_registry_index_preserved(self): + result = parse_dep("foo", {"version": "1.0", "registry-index": "https://r.example/"}) + assert result["registry"] == "https://r.example/" + + def test_table_spec_registry_name_is_rejected(self): + with pytest.raises(UnsupportedDependencyError, match="my-named-registry"): + parse_dep("foo", {"version": "1.0", "registry": "my-named-registry"}) + + def test_table_spec_registry_name_with_index_is_accepted(self): + spec = { + "version": "1.0", + "registry": "my-named-registry", + "registry-index": "https://r.example/", + } + assert parse_dep("foo", spec)["registry"] == "https://r.example/" + def test_table_spec_no_version_defaults_to_star(self): result = parse_dep("foo", {"optional": True}) assert result["req"] == "*"