Env variables: increase size limit (#11748)

Closes #7901

Author: stsewd

docs/user/environment-variables.rst

@@ -59,6 +59,12 @@ Pull Request builds
 .. with a few more scenarios,
 .. once there is better options for environment variables in config files
 
+Limitations
+-----------
+
+- Individual environment variables are limited to 48 KB in size.
+- The total size of all environment variables in a project is limited to 256 KB.
+
 Patterns of using environment variables
 ---------------------------------------
 

readthedocs/api/v3/serializers.py

@@ -31,6 +31,7 @@
     Project,
     ProjectRelationship,
 )
+from readthedocs.projects.validators import validate_environment_variable_size
 from readthedocs.redirects.constants import TYPE_CHOICES as REDIRECT_TYPE_CHOICES
 from readthedocs.redirects.models import Redirect
 from readthedocs.redirects.validators import validate_redirect
@@ -1115,7 +1116,6 @@ def get_project(self, obj):
 
 
 class EnvironmentVariableSerializer(serializers.ModelSerializer):
-    value = serializers.CharField(write_only=True)
     project = serializers.SlugRelatedField(slug_field="slug", read_only=True)
     _links = EnvironmentVariableLinksSerializer(source="*", read_only=True)
 
@@ -1131,6 +1131,25 @@ class Meta:
             "project",
             "_links",
         ]
+        extra_kwargs = {
+            "value": {"write_only": True},
+        }
+
+    def create(self, validated_data):
+        validate_environment_variable_size(
+            project=validated_data["project"],
+            new_env_value=validated_data["value"],
+            error_class=serializers.ValidationError,
+        )
+        return super().create(validated_data)
+
+    def update(self, instance, validated_data):
+        validate_environment_variable_size(
+            project=instance.project,
+            new_env_value=validated_data["value"],
+            error_class=serializers.ValidationError,
+        )
+        return super().update(instance, validated_data)
 
 
 class OrganizationLinksSerializer(BaseLinksSerializer):

readthedocs/api/v3/tests/test_environmentvariables.py

@@ -1,8 +1,11 @@
-from .mixins import APIEndpointMixin
+import django_dynamic_fixture as fixture
 from django.urls import reverse
+from django_dynamic_fixture import get
 
-import django_dynamic_fixture as fixture
 from readthedocs.projects.models import EnvironmentVariable
+from readthedocs.projects.validators import MAX_SIZE_ENV_VARS_PER_PROJECT
+
+from .mixins import APIEndpointMixin
 
 
 class EnvironmentVariablessEndpointTests(APIEndpointMixin):
@@ -154,3 +157,54 @@ def test_projects_environmentvariables_detail_delete(self):
         response = self.client.delete(url)
         self.assertEqual(response.status_code, 204)
         self.assertEqual(self.project.environmentvariable_set.count(), 0)
+
+    def test_create_large_environment_variable(self):
+        url = reverse(
+            "projects-environmentvariables-list",
+            kwargs={
+                "parent_lookup_project__slug": self.project.slug,
+            },
+        )
+        self.client.credentials(HTTP_AUTHORIZATION=f"Token {self.token.key}")
+        resp = self.client.post(
+            url,
+            data={"name": "NEWENVVAR", "value": "a" * (48000 + 1), "public": True},
+        )
+        assert resp.status_code == 400
+        assert resp.data == {
+            "value": ["Ensure this field has no more than 48000 characters."]
+        }
+
+    def test_environment_variables_total_size_per_project(self):
+        size = 2000
+        for i in range((MAX_SIZE_ENV_VARS_PER_PROJECT - size) // size):
+            get(
+                EnvironmentVariable,
+                project=self.project,
+                name=f"ENVVAR{i}",
+                value="a" * size,
+                public=False,
+            )
+
+        url = reverse(
+            "projects-environmentvariables-list",
+            kwargs={
+                "parent_lookup_project__slug": self.project.slug,
+            },
+        )
+        self.client.credentials(HTTP_AUTHORIZATION=f"Token {self.token.key}")
+
+        resp = self.client.post(
+            url,
+            data={"name": "A", "value": "a" * (size // 2), "public": True},
+        )
+        assert resp.status_code == 201
+
+        resp = self.client.post(
+            url,
+            data={"name": "B", "value": "a" * size, "public": True},
+        )
+        assert resp.status_code == 400
+        assert resp.json() == [
+            "The total size of all environment variables in the project cannot exceed 256 KB."
+        ]

readthedocs/projects/migrations/0131_increase_env_var_size.py

@@ -0,0 +1,22 @@
+# Generated by Django 4.2.16 on 2024-11-06 16:24
+
+from django.db import migrations, models
+from django_safemigrate import Safe
+
+
+class Migration(migrations.Migration):
+    safe = Safe.before_deploy
+
+    dependencies = [
+        ("projects", "0130_addons_remove_old_fields"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="environmentvariable",
+            name="value",
+            field=models.CharField(
+                help_text="Value of the environment variable", max_length=48000
+            ),
+        ),
+    ]

readthedocs/projects/models.py

@@ -53,6 +53,7 @@
     validate_custom_prefix,
     validate_custom_subproject_prefix,
     validate_domain_name,
+    validate_environment_variable_size,
     validate_no_ip,
     validate_repository_url,
 )
@@ -2065,7 +2066,7 @@ class EnvironmentVariable(TimeStampedModel, models.Model):
         help_text=_("Name of the environment variable"),
     )
     value = models.CharField(
-        max_length=2048,
+        max_length=48000,
         help_text=_("Value of the environment variable"),
     )
     project = models.ForeignKey(
@@ -2088,3 +2089,9 @@ def __str__(self):
     def save(self, *args, **kwargs):
         self.value = quote(self.value)
         return super().save(*args, **kwargs)
+
+    def clean(self):
+        validate_environment_variable_size(
+            project=self.project, new_env_value=self.value
+        )
+        return super().clean()

readthedocs/projects/validators.py

@@ -6,12 +6,16 @@
 from django.conf import settings
 from django.core.exceptions import ValidationError
 from django.core.validators import RegexValidator
+from django.db.models import Sum
+from django.db.models.functions import Length
 from django.utils.deconstruct import deconstructible
 from django.utils.html import format_html
 from django.utils.translation import gettext_lazy as _
 
 from readthedocs.projects.constants import LANGUAGES
 
+MAX_SIZE_ENV_VARS_PER_PROJECT = 256000
+
 
 @deconstructible
 class DomainNameValidator(RegexValidator):
@@ -227,3 +231,20 @@ def _clean_prefix(prefix):
     if not prefix:
         return "/"
     return f"/{prefix}/"
+
+
+def validate_environment_variable_size(
+    project, new_env_value, error_class=ValidationError
+):
+    existing_size = (
+        project.environmentvariable_set.annotate(size=Length("value")).aggregate(
+            total_size=Sum("size")
+        )["total_size"]
+        or 0
+    )
+    if existing_size + len(new_env_value) > MAX_SIZE_ENV_VARS_PER_PROJECT:
+        raise error_class(
+            _(
+                "The total size of all environment variables in the project cannot exceed 256 KB."
+            )
+        )

readthedocs/rtd_tests/tests/test_project_forms.py

@@ -35,6 +35,7 @@
     WebHookForm,
 )
 from readthedocs.projects.models import EnvironmentVariable, Project, WebHookEvent
+from readthedocs.projects.validators import MAX_SIZE_ENV_VARS_PER_PROJECT
 
 
 class TestProjectForms(TestCase):
@@ -1123,6 +1124,42 @@ def test_create(self):
             r"'string escaped here: #$\1[]{}\|'",
         )
 
+    def test_create_env_variable_with_long_value(self):
+        data = {
+            "name": "MYTOKEN",
+            "value": "a" * (48000 + 1),
+        }
+        form = EnvironmentVariableForm(data, project=self.project)
+        assert not form.is_valid()
+        assert form.errors["value"] == [
+            "Ensure this value has at most 48000 characters (it has 48001)."
+        ]
+
+    def test_create_env_variable_over_total_project_size(self):
+        size = 2000
+        for i in range((MAX_SIZE_ENV_VARS_PER_PROJECT - size) // size):
+            get(
+                EnvironmentVariable,
+                project=self.project,
+                name=f"ENVVAR{i}",
+                value="a" * size,
+                public=False,
+            )
+
+        form = EnvironmentVariableForm(
+            {"name": "A", "value": "a" * (size // 2)}, project=self.project
+        )
+        assert form.is_valid()
+        form.save()
+
+        form = EnvironmentVariableForm(
+            {"name": "B", "value": "a" * size}, project=self.project
+        )
+        assert not form.is_valid()
+        assert form.errors["__all__"] == [
+            "The total size of all environment variables in the project cannot exceed 256 KB."
+        ]
+
 
 class TestAddonsConfigForm(TestCase):
     def setUp(self):