Validate release-file URL metadata

apps/downloads/api.py

@@ -1,11 +1,13 @@
 """REST API endpoints for downloads using Tastypie and Django REST Framework."""
 
+from django.core.exceptions import ValidationError as DjangoValidationError
 from rest_framework import status, viewsets
 from rest_framework.authentication import TokenAuthentication
 from rest_framework.decorators import action
 from rest_framework.response import Response
 from tastypie import fields
 from tastypie.constants import ALL, ALL_WITH_RELATIONS
+from tastypie.validation import Validation
 
 from apps.downloads.models import OS, Release, ReleaseFile
 from apps.downloads.serializers import OSSerializer, ReleaseFileSerializer, ReleaseSerializer
@@ -14,6 +16,20 @@
 from pydotorg.resources import GenericResource, OnlyPublishedAuthorization
 
 
+class ReleaseFileValidation(Validation):
+    """Tastypie validation for release-file URL relationships."""
+
+    def is_valid(self, bundle, request=None):
+        """Return validation errors for hydrated release-file writes."""
+        if bundle.obj is None:
+            return {}
+        try:
+            bundle.obj.clean()
+        except DjangoValidationError as exc:
+            return exc.message_dict
+        return {}
+
+
 class OSResource(GenericResource):
     """Tastypie resource for operating systems."""
 
@@ -88,6 +104,7 @@ class Meta(GenericResource.Meta):
 
         queryset = ReleaseFile.objects.all()
         resource_name = "downloads/release_file"
+        validation = ReleaseFileValidation()
         fields = [
             "name",
             "slug",

apps/downloads/models.py

@@ -19,6 +19,24 @@
 from fastly.utils import purge_surrogate_key, purge_url
 
 DEFAULT_MARKUP_TYPE = getattr(settings, "DEFAULT_MARKUP_TYPE", "markdown")
+PYTHON_DOT_ORG_HTTPS_PREFIX = "https://www.python.org/"
+PYTHON_DOT_ORG_HTTP_PREFIX = "http://www.python.org/"
+RELEASE_FILE_URL_FIELDS = (
+    "url",
+    "gpg_signature_file",
+    "sigstore_signature_file",
+    "sigstore_cert_file",
+    "sigstore_bundle_file",
+    "sbom_spdx2_file",
+)
+RELEASE_FILE_SIDECAR_SUFFIXES = {
+    "gpg_signature_file": ".asc",
+    "sigstore_signature_file": ".sig",
+    "sigstore_cert_file": ".crt",
+    "sigstore_bundle_file": ".sigstore",
+    "sbom_spdx2_file": ".spdx.json",
+}
+RELEASE_FILE_HTTPS_ERROR = "Release file URLs must begin with 'https://www.python.org/'."
 
 
 class OS(ContentManageable, NameSlugModel):
@@ -361,13 +379,62 @@ def condition_url_is_blank_or_python_dot_org(column: str):
     """Conditions for a URLField column to force 'http[s]://python.org'."""
     return (
         models.Q(**{f"{column}__exact": ""})
-        | models.Q(**{f"{column}__startswith": "https://www.python.org/"})
+        | models.Q(**{f"{column}__startswith": PYTHON_DOT_ORG_HTTPS_PREFIX})
         # Older releases allowed 'http://'. 'https://' is required at
         # the API level, so shouldn't show up in newer releases.
-        | models.Q(**{f"{column}__startswith": "http://www.python.org/"})
+        | models.Q(**{f"{column}__startswith": PYTHON_DOT_ORG_HTTP_PREFIX})
     )
 
 
+def _release_file_url_values(release_file):
+    return {field_name: getattr(release_file, field_name) or "" for field_name in RELEASE_FILE_URL_FIELDS}
+
+
+def _previous_release_file_url_values(release_file):
+    if release_file.pk is None:
+        return None
+    return type(release_file).objects.filter(pk=release_file.pk).values(*RELEASE_FILE_URL_FIELDS).first()
+
+
+def _release_file_url_changed(field_name, values, previous_values):
+    if previous_values is None:
+        return True
+    return values[field_name] != (previous_values[field_name] or "")
+
+
+def _add_validation_error(errors, field_name, message):
+    errors.setdefault(field_name, []).append(message)
+
+
+def validate_release_file_urls(release_file):
+    """Validate current ReleaseFile URL writes without rejecting unchanged legacy rows."""
+    values = _release_file_url_values(release_file)
+    previous_values = _previous_release_file_url_values(release_file)
+    errors = {}
+
+    for field_name, value in values.items():
+        if not value or value.startswith(PYTHON_DOT_ORG_HTTPS_PREFIX):
+            continue
+        if _release_file_url_changed(field_name, values, previous_values):
+            _add_validation_error(errors, field_name, RELEASE_FILE_HTTPS_ERROR)
+
+    artifact_url = values["url"]
+    if artifact_url:
+        for field_name, suffix in RELEASE_FILE_SIDECAR_SUFFIXES.items():
+            sidecar_url = values[field_name]
+            expected_url = f"{artifact_url}{suffix}"
+            if not sidecar_url or sidecar_url == expected_url:
+                continue
+            _add_validation_error(
+                errors,
+                field_name,
+                f"Sidecar URL must match the artifact URL plus '{suffix}'.",
+            )
+
+    if errors:
+        raise ValidationError(errors)
+
+
 class ReleaseFile(ContentManageable, NameSlugModel):
     """Individual files in a release.
 
@@ -395,6 +462,11 @@ class ReleaseFile(ContentManageable, NameSlugModel):
     filesize = models.IntegerField(default=0)
     download_button = models.BooleanField(default=False, help_text="Use for the supernav download button for this OS")
 
+    def clean(self):
+        """Validate release-file URL relationships."""
+        super().clean()
+        validate_release_file_urls(self)
+
     def validate_unique(self, exclude=None):
         """Ensure only one release file per OS has the download button enabled."""
         if self.download_button and self.release_id:

apps/downloads/serializers.py

@@ -1,5 +1,8 @@
 """DRF serializers for the downloads API."""
 
+import copy
+
+from django.core.exceptions import ValidationError as DjangoValidationError
 from rest_framework import serializers
 
 from apps.downloads.models import OS, Release, ReleaseFile
@@ -40,6 +43,24 @@ class Meta:
 class ReleaseFileSerializer(serializers.HyperlinkedModelSerializer):
     """Serializer for release file data."""
 
+    def validate(self, attrs):
+        """Validate release-file URL relationships."""
+        attrs = super().validate(attrs)
+        release_file = self._release_file_for_validation(attrs)
+        try:
+            release_file.clean()
+        except DjangoValidationError as exc:
+            raise serializers.ValidationError(exc.message_dict) from exc
+        return attrs
+
+    def _release_file_for_validation(self, attrs):
+        if self.instance is None:
+            return ReleaseFile(**attrs)
+        release_file = copy.copy(self.instance)
+        for attr, value in attrs.items():
+            setattr(release_file, attr, value)
+        return release_file
+
     class Meta:
         """Meta configuration for ReleaseFileSerializer."""
 

apps/downloads/tests/test_models.py

@@ -1,6 +1,7 @@
 import datetime as dt
 from unittest.mock import patch
 
+from django.core.exceptions import ValidationError
 from django.db import IntegrityError, transaction
 from django.db.models import URLField
 
@@ -311,3 +312,96 @@ def test_release_file_urls_not_python_dot_org(self):
                         name="Windows installer draft",
                         **kwargs,
                     )
+
+    def test_release_file_rejects_new_http_urls(self):
+        for field in ReleaseFile._meta.get_fields():  # noqa: SLF001
+            if not isinstance(field, URLField):
+                continue
+            with self.subTest(field.name):
+                kwargs = {
+                    "url": "https://www.python.org/ftp/python/9.7.2/python-9.7.2.exe",
+                    # field.name may be 'url', but will replace the default value.
+                    field.name: "http://www.python.org/ftp/python/9.7.2/python-9.7.2.exe",
+                }
+                release_file = ReleaseFile(
+                    os=self.windows,
+                    release=self.draft_release,
+                    name="Windows installer draft",
+                    slug=f"windows-installer-draft-{field.name}",
+                    **kwargs,
+                )
+
+                with self.assertRaises(ValidationError) as cm:
+                    release_file.full_clean()
+                self.assertIn(field.name, cm.exception.message_dict)
+
+    def test_release_file_allows_existing_http_urls_to_be_edited(self):
+        release_file = ReleaseFile.objects.create(
+            os=self.windows,
+            release=self.draft_release,
+            name="Windows installer draft",
+            url="http://www.python.org/ftp/python/9.7.2/python-9.7.2.exe",
+        )
+
+        release_file.description = "Updated legacy metadata"
+
+        release_file.full_clean()
+
+    def test_release_file_rejects_existing_mismatched_sidecar_urls(self):
+        artifact_url = "https://www.python.org/ftp/python/9.7.2/Python-9.7.2-sidecar.tgz"
+        release_file = ReleaseFile.objects.create(
+            os=self.linux,
+            release=self.draft_release,
+            name="Source tarball draft",
+            slug="source-tarball-draft-mismatch",
+            url=artifact_url,
+            gpg_signature_file=artifact_url.replace("9.7.2", "9.7.1") + ".asc",
+        )
+
+        release_file.description = "Updated metadata"
+
+        with self.assertRaises(ValidationError) as cm:
+            release_file.full_clean()
+        self.assertIn("gpg_signature_file", cm.exception.message_dict)
+
+    def test_release_file_sidecar_urls_must_extend_artifact_url(self):
+        artifact_url = "https://www.python.org/ftp/python/9.7.2/Python-9.7.2-sidecar.tgz"
+        cases = {
+            "gpg_signature_file": f"{artifact_url}.asc",
+            "sigstore_signature_file": f"{artifact_url}.sig",
+            "sigstore_cert_file": f"{artifact_url}.crt",
+            "sigstore_bundle_file": f"{artifact_url}.sigstore",
+            "sbom_spdx2_file": f"{artifact_url}.spdx.json",
+        }
+
+        for field_name, expected_url in cases.items():
+            with self.subTest(field_name):
+                release_file = ReleaseFile(
+                    os=self.linux,
+                    release=self.draft_release,
+                    name="Source tarball draft",
+                    slug=f"source-tarball-draft-{field_name}",
+                    url=artifact_url,
+                    **{field_name: expected_url.replace("9.7.2", "9.7.1")},
+                )
+
+                with self.assertRaises(ValidationError) as cm:
+                    release_file.full_clean()
+                self.assertIn(field_name, cm.exception.message_dict)
+
+    def test_release_file_accepts_sidecar_urls_for_same_artifact(self):
+        artifact_url = "https://www.python.org/ftp/python/9.7.2/Python-9.7.2-sidecar.tgz"
+        release_file = ReleaseFile(
+            os=self.linux,
+            release=self.draft_release,
+            name="Source tarball draft",
+            slug="source-tarball-draft",
+            url=artifact_url,
+            gpg_signature_file=f"{artifact_url}.asc",
+            sigstore_signature_file=f"{artifact_url}.sig",
+            sigstore_cert_file=f"{artifact_url}.crt",
+            sigstore_bundle_file=f"{artifact_url}.sigstore",
+            sbom_spdx2_file=f"{artifact_url}.spdx.json",
+        )
+
+        release_file.full_clean()

apps/downloads/tests/test_views.py

@@ -412,6 +412,88 @@ def test_post_release_file(self):
         self.assertIn(data["release"], content["release"])
         self.assertEqual(content["description"], data["description"])
 
+    def test_post_release_file_rejects_http_urls(self):
+        url = self.create_url("release_file")
+        data = {
+            "name": "HTTP file",
+            "slug": "http-file",
+            "os": self.create_url("os", self.linux.pk),
+            "release": self.create_url("release", self.release_275.pk),
+            "description": "This is a description.",
+            "is_source": True,
+            "url": "http://www.python.org/ftp/python/2.7.5/Python-2.7.5-http.tgz",
+            "md5_sum": "098f6bcd4621d373cade4e832627b4f6",
+            "filesize": len("098f6bcd4621d373cade4e832627b4f6"),
+            "download_button": False,
+        }
+
+        response = self.json_client("post", url, data, HTTP_AUTHORIZATION=self.Authorization)
+
+        self.assertEqual(response.status_code, 400)
+
+    def test_post_release_file_rejects_sidecars_for_other_artifacts(self):
+        url = self.create_url("release_file")
+        artifact_url = "https://www.python.org/ftp/python/2.7.5/Python-2.7.5-api.tgz"
+        data = {
+            "name": "File with wrong sidecar",
+            "slug": "file-with-wrong-sidecar",
+            "os": self.create_url("os", self.linux.pk),
+            "release": self.create_url("release", self.release_275.pk),
+            "description": "This is a description.",
+            "is_source": True,
+            "url": artifact_url,
+            "gpg_signature_file": "https://www.python.org/ftp/python/2.7.5/Python-2.7.4-api.tgz.asc",
+            "md5_sum": "098f6bcd4621d373cade4e832627b4f6",
+            "filesize": len("098f6bcd4621d373cade4e832627b4f6"),
+            "download_button": False,
+        }
+
+        response = self.json_client("post", url, data, HTTP_AUTHORIZATION=self.Authorization)
+
+        self.assertEqual(response.status_code, 400)
+
+    def test_update_release_file_rejects_changed_http_urls(self):
+        url = self.create_url("release_file", self.release_275_linux.pk)
+        data = {
+            "name": self.release_275_linux.name,
+            "slug": self.release_275_linux.slug,
+            "os": self.create_url("os", self.linux.pk),
+            "release": self.create_url("release", self.release_275.pk),
+            "description": self.release_275_linux.description,
+            "is_source": self.release_275_linux.is_source,
+            "url": "http://www.python.org/ftp/python/2.7.5/Python-2.7.5.tgz",
+            "md5_sum": self.release_275_linux.md5_sum,
+            "sha256_sum": self.release_275_linux.sha256_sum,
+            "filesize": self.release_275_linux.filesize,
+            "download_button": self.release_275_linux.download_button,
+        }
+
+        response = self.json_client("put", url, data, HTTP_AUTHORIZATION=self.Authorization)
+
+        self.assertEqual(response.status_code, 400)
+
+    def test_update_release_file_rejects_changed_sidecars_for_other_artifacts(self):
+        url = self.create_url("release_file", self.release_275_linux.pk)
+        artifact_url = self.release_275_linux.url
+        data = {
+            "name": self.release_275_linux.name,
+            "slug": self.release_275_linux.slug,
+            "os": self.create_url("os", self.linux.pk),
+            "release": self.create_url("release", self.release_275.pk),
+            "description": self.release_275_linux.description,
+            "is_source": self.release_275_linux.is_source,
+            "url": artifact_url,
+            "gpg_signature_file": artifact_url.replace("2.7.5", "2.7.4") + ".asc",
+            "md5_sum": self.release_275_linux.md5_sum,
+            "sha256_sum": self.release_275_linux.sha256_sum,
+            "filesize": self.release_275_linux.filesize,
+            "download_button": self.release_275_linux.download_button,
+        }
+
+        response = self.json_client("put", url, data, HTTP_AUTHORIZATION=self.Authorization)
+
+        self.assertEqual(response.status_code, 400)
+
     def test_delete_release_file(self):
         url = self.create_url("release_file", self.release_275_linux.pk)
         response = self.json_client("delete", url)