From b0a885676f24c18fdc23df1e46d58e0b99cd4f2b Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Fri, 16 Oct 2026 15:53:46 -0400 Subject: [PATCH 1/2] Sign pull-through links instead of trusting the redirect url Pull-through links on the simple pages used ?redirect=, and the content app fetched whatever url the client sent using the remote's credentials and proxy. Links now carry a signed token bound to the remote, filename, upstream url and sha256, and pull-through downloads are validated against that sha256. Co-Authored-By: Claude Opus 5.5 --- CHANGES/+pull-through-signed-urls.bugfix | 1 + pulp_python/app/models.py | 44 +++++++++----- pulp_python/app/pypi/views.py | 11 +++- pulp_python/app/utils.py | 36 +++++++++++ .../tests/functional/api/test_full_mirror.py | 59 +++++++++++++++---- pulp_python/tests/unit/test_pull_through.py | 58 ++++++++++++++++++ 6 files changed, 180 insertions(+), 29 deletions(-) create mode 100644 CHANGES/+pull-through-signed-urls.bugfix create mode 100644 pulp_python/tests/unit/test_pull_through.py diff --git a/CHANGES/+pull-through-signed-urls.bugfix b/CHANGES/+pull-through-signed-urls.bugfix new file mode 100644 index 00000000..804b9268 --- /dev/null +++ b/CHANGES/+pull-through-signed-urls.bugfix @@ -0,0 +1 @@ +Fixed pull-through downloads trusting the upstream url given by the client; pull-through links are now signed and downloads are checked against the upstream sha256. diff --git a/pulp_python/app/models.py b/pulp_python/app/models.py index 902bf5a8..d807d663 100644 --- a/pulp_python/app/models.py +++ b/pulp_python/app/models.py @@ -34,12 +34,14 @@ from .provenance import Provenance from .utils import ( + PULL_THROUGH_PARAM, PYPI_LAST_SERIAL, PYPI_SERIAL_CONSTANT, artifact_to_metadata_artifact, artifact_to_python_content_data, canonicalize_name, python_content_to_json, + unsign_pull_through_token, ) log = getLogger(__name__) @@ -357,30 +359,42 @@ class PythonRemote(Remote, AutoAddObjPermsMixin): vulnerabilities = models.BooleanField(default=False) def get_remote_artifact_url(self, relative_path=None, request=None): - """Get url for remote_artifact""" - if request and (url := request.query.get("redirect")): - # This is a special case for pull-through caching - # To handle PEP 658, it states that if the package has metadata available then it - # should be found at the download URL + ".metadata". Thus if the request url ends with - # ".metadata" then we need to add ".metadata" to the redirect url if not present. - if relative_path: - if relative_path.endswith(".metadata") and not url.endswith(".metadata"): - url += ".metadata" - # Handle special case for bug in pip (TODO file issue in pip) where it appends - # ".metadata" to the redirect url instead of the request url - if url.endswith(".metadata") and not relative_path.endswith(".metadata"): - setattr(self, "_real_relative_path", url.rsplit("/", 1)[1]) + """Get url for remote_artifact, content app requests need a signed pull-through token.""" + if request is None: + return super().get_remote_artifact_url(relative_path) + if not (token := request.query.get(PULL_THROUGH_PARAM)): + return None + filename = PurePath(relative_path or "").name + # pip adds ".metadata" to the end of the url (the token) instead of the url path + if token.endswith(".metadata"): + token = token.removesuffix(".metadata") + if not filename.endswith(".metadata"): + filename = f"{filename}.metadata" + setattr(self, "_real_relative_path", filename) + if resolved := unsign_pull_through_token(self, token, filename): + url, sha256 = resolved + if sha256: + self.__dict__.setdefault("_pull_through_digests", {})[url] = sha256 return url - return super().get_remote_artifact_url(relative_path, request=request) + return None def get_remote_artifact_content_type(self, relative_path=None): """Return PythonPackageContent, except for metadata artifacts.""" if hasattr(self, "_real_relative_path"): relative_path = getattr(self, "_real_relative_path") - if relative_path and relative_path.endswith(".whl.metadata"): + if relative_path and relative_path.endswith(".metadata"): return None return PythonPackageContent + def get_downloader(self, remote_artifact=None, url=None, download_factory=None, **kwargs): + """Validate pull-through downloads against the sha256 from their signed path.""" + digests = self.__dict__.get("_pull_through_digests", {}) + if remote_artifact and not remote_artifact.sha256: + remote_artifact.sha256 = digests.get(remote_artifact.url) + return super().get_downloader( + remote_artifact=remote_artifact, url=url, download_factory=download_factory, **kwargs + ) + class Meta: default_related_name = "%(app_label)s_%(model_name)s" permissions = [ diff --git a/pulp_python/app/pypi/views.py b/pulp_python/app/pypi/views.py index 8d580a2f..c80c3772 100644 --- a/pulp_python/app/pypi/views.py +++ b/pulp_python/app/pypi/views.py @@ -56,6 +56,7 @@ get_remote_package_filter, get_remote_simple_page, python_content_to_json, + sign_pull_through_path, write_simple_detail, write_simple_detail_json, write_simple_index, @@ -371,8 +372,14 @@ def pull_through_package_simple(self, package, path, remote): def parse_package(release_package): parsed = urlparse(release_package.url) stripped_url = urlunsplit(chain(parsed[:3], ("", ""))) - redirect_path = f"{path}/{release_package.filename}?redirect={stripped_url}" - d_url = urljoin(self.base_content_url, redirect_path) + pt_path = sign_pull_through_path( + remote, + release_package.filename, + stripped_url, + sha256=release_package.digests.get("sha256"), + metadata_sha256=(release_package.metadata_digests or {}).get("sha256"), + ) + d_url = urljoin(self.base_content_url, f"{path}/{pt_path}") return { "filename": release_package.filename, "url": d_url, diff --git a/pulp_python/app/utils.py b/pulp_python/app/utils.py index 8838fa51..806783a0 100644 --- a/pulp_python/app/utils.py +++ b/pulp_python/app/utils.py @@ -12,6 +12,7 @@ import pkginfo from aiohttp.client_exceptions import ClientError from django.conf import settings +from django.core import signing from django.db.models import F, FilteredRelation, Q from django.db.utils import IntegrityError from jinja2 import Template @@ -35,6 +36,10 @@ SUPPORTED_METADATA_VERSIONS = ("1.0", "1.1", "1.2", "2.0", "2.1", "2.2", "2.3", "2.4", "2.5") SIMPLE_API_VERSION = "1.1" + +PULL_THROUGH_PARAM = "pt" +PULL_THROUGH_SALT = "pulp_python.pull_through" + PYPI_SIMPLE_V1_HTML = "application/vnd.pypi.simple.v1+html" PYPI_SIMPLE_V1_JSON = "application/vnd.pypi.simple.v1+json" @@ -768,3 +773,34 @@ async def aget_remote_simple_page(package, remote, max_retries=1): else: page = ProjectPage.from_html(package, open(d.path, "rb").read(), base_url=url) return page + + +def sign_pull_through_path(remote, filename, url, sha256=None, metadata_sha256=None): + """Returns the content app path of a pull-through package, carrying its signed upstream url.""" + data = {"r": str(remote.pk), "f": filename, "u": url} + if sha256: + data["s"] = sha256 + if metadata_sha256: + data["m"] = metadata_sha256 + token = signing.Signer(salt=PULL_THROUGH_SALT).sign_object(data, compress=True) + return f"{filename}?{PULL_THROUGH_PARAM}={token}" + + +def unsign_pull_through_token(remote, token, filename): + """ + Returns the upstream (url, sha256) for a file of a signed pull-through token. + + filename can be the signed package's filename or its PEP 658 metadata file. Returns None if + the token is invalid, was signed for another remote or doesn't cover filename. + """ + try: + data = signing.Signer(salt=PULL_THROUGH_SALT).unsign_object(token) + except signing.BadSignature: + return None + if data.get("r") != str(remote.pk): + return None + if filename == data["f"]: + return data["u"], data.get("s") + if filename == f"{data['f']}.metadata" and data.get("m"): + return f"{data['u']}.metadata", data["m"] + return None diff --git a/pulp_python/tests/functional/api/test_full_mirror.py b/pulp_python/tests/functional/api/test_full_mirror.py index e89cdac2..aa5dfec6 100644 --- a/pulp_python/tests/functional/api/test_full_mirror.py +++ b/pulp_python/tests/functional/api/test_full_mirror.py @@ -54,7 +54,7 @@ def test_pull_through_simple(python_remote_factory, python_distribution_factory, assert len(project_page.packages) == 2 for package in project_page.packages: assert package.filename in PYTHON_XS_FIXTURE_CHECKSUMS - relative_path = f"{distro.base_path}/{package.filename}?redirect=" + relative_path = f"{distro.base_path}/{package.filename}?pt=" assert urljoin(pulp_content_url, relative_path) in package.url assert PYTHON_XS_FIXTURE_CHECKSUMS[package.filename] == package.digests["sha256"] @@ -134,7 +134,7 @@ def test_pull_through_with_repo( url = urljoin(distro.base_url, "simple/aiohttp/") project_page = ProjectPage.from_response(requests.get(url), "aiohttp") for package in sample(project_page.packages, 3): - assert "?redirect=" in package.url + assert "?pt=" in package.url r = requests.get(package.url) assert r.status_code == 200 @@ -172,12 +172,12 @@ def test_pull_through_local_only( url = f"{distro.base_url}simple/pulpcore/" r = requests.get(url) assert r.status_code == 200 - assert "?redirect=" not in r.text + assert "?pt=" not in r.text url = f"{distro.base_url}simple/shelf-reader/" r = requests.get(url) assert r.status_code == 200 - assert "?redirect=" in r.text + assert "?pt=" in r.text url = f"{distro.base_url}simple/pulp_python/" r = requests.get(url) @@ -210,9 +210,8 @@ def test_pull_through_metadata(python_remote_factory, python_distribution_factor """ Tests that metadata is correctly served when using pull-through. - So when requesting the metadata url according to PEP 658 you should just need to add .metadata - to the end of the url path. Since pull-through includes a redirect query parameter we need to - test adding .metadata to the end of the url path vs adding it to the end of redirect query. + According to PEP 658 the metadata url is the package url with .metadata added to its path. + pip and uv build it differently (string vs path append), both have to work. """ remote = python_remote_factory(includes=["pytz"]) distro = python_distribution_factory(remote=remote.pulp_href) @@ -226,20 +225,56 @@ def test_pull_through_metadata(python_remote_factory, python_distribution_factor assert package1.has_metadata assert package2.has_metadata - # The correct way to get the metadata url: add to path (uv does this) + # Add to path (uv does this) parts1 = urlsplit(package1.url) url1 = urlunsplit((parts1[0], parts1[1], parts1[2] + ".metadata", parts1[3], parts1[4])) r = requests.get(url1) assert r.status_code == 200 assert sha256(r.content).hexdigest() == package1.metadata_digests["sha256"] - # The incorrect way to get the metadata url: add to end of string (pip does this) + # Add to end of string (pip does this) url2 = package2.url + ".metadata" r = requests.get(url2) assert r.status_code == 200 assert sha256(r.content).hexdigest() == package2.metadata_digests["sha256"] +@pytest.mark.parallel +def test_pull_through_rejects_bad_links(python_remote_factory, python_distribution_factory): + """Tests that pull-through only downloads files from links signed for the distro's remote.""" + remote = python_remote_factory(includes=["shelf-reader"]) + distro = python_distribution_factory(remote=remote.pulp_href) + remote2 = python_remote_factory(includes=["shelf-reader"]) + distro2 = python_distribution_factory(remote=remote2.pulp_href) + + url = f"{distro.base_url}simple/shelf-reader/" + package1, package2 = ProjectPage.from_response(requests.get(url), "shelf-reader").packages + token1 = package1.url.split("?pt=")[1] + + # Tampered token + tampered = token1[:-1] + ("A" if token1[-1] != "A" else "B") + r = requests.get(package1.url.replace(token1, tampered)) + assert r.status_code == 404 + + # Token used for another file + r = requests.get(package2.url.replace(package2.url.split("?pt=")[1], token1)) + assert r.status_code == 404 + + # Token used on a distribution with another remote + r = requests.get(package1.url.replace(f"/{distro.base_path}/", f"/{distro2.base_path}/")) + assert r.status_code == 404 + + # Old unsigned redirect links + upstream_url = urljoin(remote.url, "simple/shelf-reader/") + upstream = ProjectPage.from_response(requests.get(upstream_url), "shelf-reader").packages[0] + r = requests.get(f"{distro.base_url}{upstream.filename}?redirect={upstream.url}") + assert r.status_code == 404 + + r = requests.get(package1.url) + assert r.status_code == 200 + assert sha256(r.content).hexdigest() == package1.digests["sha256"] + + @pytest.mark.parallel def test_pull_through_metadata_with_repo( python_repo_factory, @@ -257,7 +292,7 @@ def test_pull_through_metadata_with_repo( filename = "pip-26.0.1-py3-none-any.whl" package = next(p for p in project_page.packages if p.filename == filename) assert package.has_metadata - assert "?redirect=" in package.url + assert "?pt=" in package.url # Retrieve the metadata and assert the content was not saved to the repository parts = urlsplit(package.url) @@ -268,7 +303,7 @@ def test_pull_through_metadata_with_repo( project_page = ProjectPage.from_response(requests.get(pip_url), "pip") package = next(p for p in project_page.packages if p.filename == filename) assert package.has_metadata - assert "?redirect=" in package.url + assert "?pt=" in package.url # Now retrieve the package and assert the content was saved with metadata r = requests.get(package.url) @@ -281,7 +316,7 @@ def test_pull_through_metadata_with_repo( # Check the simple page is updated to point to the local repository project_page = ProjectPage.from_response(requests.get(pip_url), "pip") package = next(p for p in project_page.packages if p.filename == filename) - assert "?redirect=" not in package.url + assert "?pt=" not in package.url r = requests.get(package.metadata_url) assert r.status_code == 200 assert sha256(r.content).hexdigest() == package.metadata_digests["sha256"] diff --git a/pulp_python/tests/unit/test_pull_through.py b/pulp_python/tests/unit/test_pull_through.py new file mode 100644 index 00000000..46401abb --- /dev/null +++ b/pulp_python/tests/unit/test_pull_through.py @@ -0,0 +1,58 @@ +from types import SimpleNamespace +from uuid import uuid4 + +from pulp_python.app.utils import ( + PULL_THROUGH_PARAM, + sign_pull_through_path, + unsign_pull_through_token, +) + +FILENAME = "shelf_reader-0.1-py2-none-any.whl" +URL = f"https://files.example.com/packages/ab/cd/{FILENAME}" + + +def _token(remote, **kwargs): + path = sign_pull_through_path(remote, FILENAME, URL, **kwargs) + filename, token = path.split(f"?{PULL_THROUGH_PARAM}=") + assert filename == FILENAME + return token + + +def test_round_trip(): + remote = SimpleNamespace(pk=uuid4()) + token = _token(remote, sha256="a" * 64, metadata_sha256="b" * 64) + + assert unsign_pull_through_token(remote, token, FILENAME) == (URL, "a" * 64) + assert unsign_pull_through_token(remote, token, f"{FILENAME}.metadata") == ( + f"{URL}.metadata", + "b" * 64, + ) + + +def test_metadata_requires_signed_digest(): + remote = SimpleNamespace(pk=uuid4()) + token = _token(remote, sha256="a" * 64) + + assert unsign_pull_through_token(remote, token, f"{FILENAME}.metadata") is None + + +def test_rejects_other_remote(): + token = _token(SimpleNamespace(pk=uuid4()), sha256="a" * 64) + + assert unsign_pull_through_token(SimpleNamespace(pk=uuid4()), token, FILENAME) is None + + +def test_rejects_other_filename(): + remote = SimpleNamespace(pk=uuid4()) + token = _token(remote, sha256="a" * 64) + + assert unsign_pull_through_token(remote, token, "evil-1.0-py3-none-any.whl") is None + + +def test_rejects_tampered_token(): + remote = SimpleNamespace(pk=uuid4()) + token = _token(remote, sha256="a" * 64) + tampered = token[:-1] + ("A" if token[-1] != "A" else "B") + + assert unsign_pull_through_token(remote, tampered, FILENAME) is None + assert unsign_pull_through_token(remote, "not-a-token", FILENAME) is None From fd55068236fa6efe533956e44e7664d307069e6c Mon Sep 17 00:00:00 2001 From: Gerrod Ubben Date: Fri, 16 Oct 2026 16:33:22 -0400 Subject: [PATCH 2/2] Use plain filename pull-through links when Redis is available pip appends .metadata to the whole link, so with a query string the wheel and its metadata share one request path and the content cache can serve one for the other. When Redis is available, the simple page now stores each file's upstream url and digests under the remote and links to the plain filename. Signed ?pt= links remain the fallback without Redis. fixes #1428 Co-Authored-By: Claude Opus 5.5 --- CHANGES/1428.bugfix | 1 + pulp_python/app/models.py | 25 ++++--- pulp_python/app/pypi/views.py | 42 +++++++---- pulp_python/app/utils.py | 55 ++++++++++++++ .../tests/functional/api/test_full_mirror.py | 71 ++++++++++++++++--- pulp_python/tests/unit/test_pull_through.py | 29 ++++++++ 6 files changed, 189 insertions(+), 34 deletions(-) create mode 100644 CHANGES/1428.bugfix diff --git a/CHANGES/1428.bugfix b/CHANGES/1428.bugfix new file mode 100644 index 00000000..b5bf381f --- /dev/null +++ b/CHANGES/1428.bugfix @@ -0,0 +1 @@ +Fixed pip intermittently receiving a pull-through wheel instead of its metadata, or the reverse, when Redis caching is enabled. diff --git a/pulp_python/app/models.py b/pulp_python/app/models.py index d807d663..fc57cd6e 100644 --- a/pulp_python/app/models.py +++ b/pulp_python/app/models.py @@ -40,6 +40,7 @@ artifact_to_metadata_artifact, artifact_to_python_content_data, canonicalize_name, + lookup_pull_through_entry, python_content_to_json, unsign_pull_through_token, ) @@ -359,19 +360,21 @@ class PythonRemote(Remote, AutoAddObjPermsMixin): vulnerabilities = models.BooleanField(default=False) def get_remote_artifact_url(self, relative_path=None, request=None): - """Get url for remote_artifact, content app requests need a signed pull-through token.""" + """Get url for remote_artifact, content app requests need a stored or signed entry.""" if request is None: return super().get_remote_artifact_url(relative_path) - if not (token := request.query.get(PULL_THROUGH_PARAM)): - return None filename = PurePath(relative_path or "").name - # pip adds ".metadata" to the end of the url (the token) instead of the url path - if token.endswith(".metadata"): - token = token.removesuffix(".metadata") - if not filename.endswith(".metadata"): - filename = f"{filename}.metadata" - setattr(self, "_real_relative_path", filename) - if resolved := unsign_pull_through_token(self, token, filename): + if token := request.query.get(PULL_THROUGH_PARAM): + # pip adds ".metadata" to the end of the url (the token) instead of the url path + if token.endswith(".metadata"): + token = token.removesuffix(".metadata") + if not filename.endswith(".metadata"): + filename = f"{filename}.metadata" + setattr(self, "_real_relative_path", filename) + resolved = unsign_pull_through_token(self, token, filename) + else: + resolved = lookup_pull_through_entry(self, filename) + if resolved: url, sha256 = resolved if sha256: self.__dict__.setdefault("_pull_through_digests", {})[url] = sha256 @@ -387,7 +390,7 @@ def get_remote_artifact_content_type(self, relative_path=None): return PythonPackageContent def get_downloader(self, remote_artifact=None, url=None, download_factory=None, **kwargs): - """Validate pull-through downloads against the sha256 from their signed path.""" + """Validate pull-through downloads against the sha256 from their pull-through entry.""" digests = self.__dict__.get("_pull_through_digests", {}) if remote_artifact and not remote_artifact.sha256: remote_artifact.sha256 = digests.get(remote_artifact.url) diff --git a/pulp_python/app/pypi/views.py b/pulp_python/app/pypi/views.py index c80c3772..782d8325 100644 --- a/pulp_python/app/pypi/views.py +++ b/pulp_python/app/pypi/views.py @@ -57,6 +57,7 @@ get_remote_simple_page, python_content_to_json, sign_pull_through_path, + store_pull_through_entries, write_simple_detail, write_simple_detail_json, write_simple_index, @@ -369,16 +370,21 @@ def list(self, request, path): def pull_through_package_simple(self, package, path, remote): """Gets the package's simple page from remote.""" - def parse_package(release_package): + def upstream_url(release_package): parsed = urlparse(release_package.url) - stripped_url = urlunsplit(chain(parsed[:3], ("", ""))) - pt_path = sign_pull_through_path( - remote, - release_package.filename, - stripped_url, - sha256=release_package.digests.get("sha256"), - metadata_sha256=(release_package.metadata_digests or {}).get("sha256"), - ) + return urlunsplit(chain(parsed[:3], ("", ""))) + + def parse_package(release_package, stored): + if stored: + pt_path = release_package.filename + else: + pt_path = sign_pull_through_path( + remote, + release_package.filename, + upstream_url(release_package), + sha256=release_package.digests.get("sha256"), + metadata_sha256=(release_package.metadata_digests or {}).get("sha256"), + ) d_url = urljoin(self.base_content_url, f"{path}/{pt_path}") return { "filename": release_package.filename, @@ -402,11 +408,19 @@ def parse_package(release_package): if not page: log.info(f"Failed to fetch {package} simple page from {remote.url}") return {} - return { - p.filename: parse_package(p) - for p in page.packages - if rfilter.filter_release(package, p.version) - } + packages = [p for p in page.packages if rfilter.filter_release(package, p.version)] + stored = store_pull_through_entries( + remote, + { + p.filename: { + "url": upstream_url(p), + "sha256": p.digests.get("sha256"), + "metadata_sha256": (p.metadata_digests or {}).get("sha256"), + } + for p in packages + }, + ) + return {p.filename: parse_package(p, stored) for p in packages} @extend_schema(operation_id="pypi_simple_package_read", summary="Get package simple page") @method_decorator(cache_control(max_age=600, public=True)) diff --git a/pulp_python/app/utils.py b/pulp_python/app/utils.py index 806783a0..06c10592 100644 --- a/pulp_python/app/utils.py +++ b/pulp_python/app/utils.py @@ -21,6 +21,7 @@ from packaging.version import InvalidVersion, parse from pypi_simple import ACCEPT_JSON_PREFERRED, ProjectPage +from pulpcore.plugin.cache import SyncContentCache from pulpcore.plugin.exceptions import TimeoutException from pulpcore.plugin.models import Artifact, Remote, VulnerabilityReport from pulpcore.plugin.util import get_domain @@ -39,6 +40,8 @@ PULL_THROUGH_PARAM = "pt" PULL_THROUGH_SALT = "pulp_python.pull_through" +# Outlive both the cached simple pages and their max-age in client caches +PULL_THROUGH_TTL = settings.CACHE_SETTINGS["EXPIRES_TTL"] + 600 PYPI_SIMPLE_V1_HTML = "application/vnd.pypi.simple.v1+html" PYPI_SIMPLE_V1_JSON = "application/vnd.pypi.simple.v1+json" @@ -804,3 +807,55 @@ def unsign_pull_through_token(remote, token, filename): if filename == f"{data['f']}.metadata" and data.get("m"): return f"{data['u']}.metadata", data["m"] return None + + +def _pull_through_key(remote): + # Editing the remote invalidates its entries + return f"pulp_python:pull_through:{remote.pk}:{remote.pulp_last_updated.timestamp()}" + + +def store_pull_through_entries(remote, entries): + """ + Stores filename -> {url, sha256, metadata_sha256} entries of a remote in Redis. + + Returns True if the entries were stored, so plain filename links can be used for them. + """ + if not settings.CACHE_ENABLED: + return False + if not entries: + return True + key = _pull_through_key(remote) + try: + redis = SyncContentCache().redis + redis.hset(key, mapping={f: json.dumps(e) for f, e in entries.items()}) + redis.expire(key, PULL_THROUGH_TTL) + except Exception as e: + log.warning("Failed to store pull-through entries: %s", e) + return False + return True + + +def lookup_pull_through_entry(remote, filename): + """ + Returns the upstream (url, sha256) for a file stored by store_pull_through_entries. + + filename can be a stored package's filename or its PEP 658 metadata file. + """ + if not settings.CACHE_ENABLED: + return None + is_metadata = filename.endswith(".metadata") + try: + raw = SyncContentCache().redis.hget( + _pull_through_key(remote), filename.removesuffix(".metadata") + ) + except Exception as e: + log.warning("Failed to look up pull-through entry: %s", e) + return None + if not raw: + return None + entry = json.loads(raw) + if not is_metadata: + return entry["url"], entry.get("sha256") + if entry.get("metadata_sha256"): + return f"{entry['url']}.metadata", entry["metadata_sha256"] + return None diff --git a/pulp_python/tests/functional/api/test_full_mirror.py b/pulp_python/tests/functional/api/test_full_mirror.py index aa5dfec6..c942038e 100644 --- a/pulp_python/tests/functional/api/test_full_mirror.py +++ b/pulp_python/tests/functional/api/test_full_mirror.py @@ -16,6 +16,12 @@ ) +@pytest.fixture +def signed_links(pulp_settings): + """Pull-through links carry a signed token only when Redis is not available.""" + return not pulp_settings.CACHE_ENABLED + + def test_pull_through_install( python_bindings, python_remote_factory, python_distribution_factory, delete_orphans_pre ): @@ -43,7 +49,9 @@ def test_pull_through_install( @pytest.mark.parallel -def test_pull_through_simple(python_remote_factory, python_distribution_factory, pulp_content_url): +def test_pull_through_simple( + python_remote_factory, python_distribution_factory, pulp_content_url, signed_links +): """Tests that the simple page is properly modified when requesting a pull-through.""" remote = python_remote_factory(url=PYPI_URL, includes=["shelf-reader"]) distro = python_distribution_factory(remote=remote.pulp_href) @@ -54,8 +62,11 @@ def test_pull_through_simple(python_remote_factory, python_distribution_factory, assert len(project_page.packages) == 2 for package in project_page.packages: assert package.filename in PYTHON_XS_FIXTURE_CHECKSUMS - relative_path = f"{distro.base_path}/{package.filename}?pt=" - assert urljoin(pulp_content_url, relative_path) in package.url + expected = urljoin(pulp_content_url, f"{distro.base_path}/{package.filename}") + if signed_links: + assert package.url.startswith(f"{expected}?pt=") + else: + assert package.url == expected assert PYTHON_XS_FIXTURE_CHECKSUMS[package.filename] == package.digests["sha256"] @@ -124,6 +135,7 @@ def test_pull_through_with_repo( pulpcore_bindings, monitor_task, has_pulp_plugin, + signed_links, ): """Tests that content is saved to repository.""" remote = python_remote_factory(includes=[]) @@ -134,7 +146,7 @@ def test_pull_through_with_repo( url = urljoin(distro.base_url, "simple/aiohttp/") project_page = ProjectPage.from_response(requests.get(url), "aiohttp") for package in sample(project_page.packages, 3): - assert "?pt=" in package.url + assert ("?pt=" in package.url) == signed_links r = requests.get(package.url) assert r.status_code == 200 @@ -161,7 +173,7 @@ def test_pull_through_with_repo( @pytest.mark.parallel def test_pull_through_local_only( - python_remote_factory, python_distribution_factory, python_repo_with_sync + python_remote_factory, python_distribution_factory, python_repo_with_sync, signed_links ): """Tests that pull-through checks the repository if the package is not present on the remote.""" remote = python_remote_factory(url=PYPI_URL, includes=["pulpcore"]) @@ -177,7 +189,8 @@ def test_pull_through_local_only( url = f"{distro.base_url}simple/shelf-reader/" r = requests.get(url) assert r.status_code == 200 - assert "?pt=" in r.text + assert "shelf-reader-0.1.tar.gz" in r.text + assert ("?pt=" in r.text) == signed_links url = f"{distro.base_url}simple/pulp_python/" r = requests.get(url) @@ -240,8 +253,46 @@ def test_pull_through_metadata(python_remote_factory, python_distribution_factor @pytest.mark.parallel -def test_pull_through_rejects_bad_links(python_remote_factory, python_distribution_factory): +def test_pull_through_rejects_unknown_files( + python_remote_factory, python_distribution_factory, signed_links +): + """Tests that pull-through only downloads files listed on a simple page of the distro's remote.""" + if signed_links: + pytest.skip("Pull-through links are signed when Redis is not available") + remote = python_remote_factory(includes=["shelf-reader"]) + distro = python_distribution_factory(remote=remote.pulp_href) + remote2 = python_remote_factory(includes=["shelf-reader"]) + distro2 = python_distribution_factory(remote=remote2.pulp_href) + + # Not listed on any simple page yet + r = requests.get(f"{distro.base_url}shelf-reader-0.1.tar.gz") + assert r.status_code == 404 + + url = f"{distro.base_url}simple/shelf-reader/" + package = ProjectPage.from_response(requests.get(url), "shelf-reader").packages[0] + + # Listed on a simple page of another remote + r = requests.get(package.url.replace(f"/{distro.base_path}/", f"/{distro2.base_path}/")) + assert r.status_code == 404 + + # Old unsigned redirect links + upstream_url = urljoin(remote.url, "simple/shelf-reader/") + upstream = ProjectPage.from_response(requests.get(upstream_url), "shelf-reader").packages[0] + r = requests.get(f"{distro.base_url}shelf_reader-9.9.tar.gz?redirect={upstream.url}") + assert r.status_code == 404 + + r = requests.get(package.url) + assert r.status_code == 200 + assert sha256(r.content).hexdigest() == package.digests["sha256"] + + +@pytest.mark.parallel +def test_pull_through_rejects_bad_links( + python_remote_factory, python_distribution_factory, signed_links +): """Tests that pull-through only downloads files from links signed for the distro's remote.""" + if not signed_links: + pytest.skip("Pull-through links are only signed when Redis is not available") remote = python_remote_factory(includes=["shelf-reader"]) distro = python_distribution_factory(remote=remote.pulp_href) remote2 = python_remote_factory(includes=["shelf-reader"]) @@ -281,6 +332,7 @@ def test_pull_through_metadata_with_repo( python_remote_factory, python_distribution_factory, pulpcore_bindings, + signed_links, ): """Tests that metadata is correctly saved when using pull-through with a repository.""" remote = python_remote_factory(url=PYPI_URL, includes=["pip"]) @@ -292,7 +344,7 @@ def test_pull_through_metadata_with_repo( filename = "pip-26.0.1-py3-none-any.whl" package = next(p for p in project_page.packages if p.filename == filename) assert package.has_metadata - assert "?pt=" in package.url + assert ("?pt=" in package.url) == signed_links # Retrieve the metadata and assert the content was not saved to the repository parts = urlsplit(package.url) @@ -303,11 +355,12 @@ def test_pull_through_metadata_with_repo( project_page = ProjectPage.from_response(requests.get(pip_url), "pip") package = next(p for p in project_page.packages if p.filename == filename) assert package.has_metadata - assert "?pt=" in package.url + assert ("?pt=" in package.url) == signed_links # Now retrieve the package and assert the content was saved with metadata r = requests.get(package.url) assert r.status_code == 200 + assert sha256(r.content).hexdigest() == package.digests["sha256"] pa = pulpcore_bindings.ArtifactsApi.list(sha256=package.digests["sha256"]) assert pa.count == 1 ma = pulpcore_bindings.ArtifactsApi.list(sha256=package.metadata_digests["sha256"]) diff --git a/pulp_python/tests/unit/test_pull_through.py b/pulp_python/tests/unit/test_pull_through.py index 46401abb..9520d45c 100644 --- a/pulp_python/tests/unit/test_pull_through.py +++ b/pulp_python/tests/unit/test_pull_through.py @@ -1,9 +1,15 @@ +from datetime import datetime, timedelta from types import SimpleNamespace from uuid import uuid4 +import pytest +from django.conf import settings + from pulp_python.app.utils import ( PULL_THROUGH_PARAM, + lookup_pull_through_entry, sign_pull_through_path, + store_pull_through_entries, unsign_pull_through_token, ) @@ -56,3 +62,26 @@ def test_rejects_tampered_token(): assert unsign_pull_through_token(remote, tampered, FILENAME) is None assert unsign_pull_through_token(remote, "not-a-token", FILENAME) is None + + +@pytest.mark.skipif(not settings.CACHE_ENABLED, reason="CACHE_ENABLED is not set") +def test_stored_entries(): + remote = SimpleNamespace(pk=uuid4(), pulp_last_updated=datetime.now()) + entry = {"url": URL, "sha256": "a" * 64, "metadata_sha256": None} + assert store_pull_through_entries(remote, {FILENAME: entry}) + + assert lookup_pull_through_entry(remote, FILENAME) == (URL, "a" * 64) + assert lookup_pull_through_entry(remote, f"{FILENAME}.metadata") is None + assert lookup_pull_through_entry(remote, "evil-1.0-py3-none-any.whl") is None + + entry["metadata_sha256"] = "b" * 64 + assert store_pull_through_entries(remote, {FILENAME: entry}) + assert lookup_pull_through_entry(remote, f"{FILENAME}.metadata") == ( + f"{URL}.metadata", + "b" * 64, + ) + + other = SimpleNamespace(pk=uuid4(), pulp_last_updated=remote.pulp_last_updated) + assert lookup_pull_through_entry(other, FILENAME) is None + edited = SimpleNamespace(pk=remote.pk, pulp_last_updated=datetime.now() + timedelta(1)) + assert lookup_pull_through_entry(edited, FILENAME) is None