diff --git a/docs/user/environment-variables.rst b/docs/user/environment-variables.rst index 70e4456f7c6..4cfc00fe90e 100644 --- a/docs/user/environment-variables.rst +++ b/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 --------------------------------------- diff --git a/readthedocs/api/v3/serializers.py b/readthedocs/api/v3/serializers.py index cb635580b1d..b271cf255fd 100644 --- a/readthedocs/api/v3/serializers.py +++ b/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): diff --git a/readthedocs/api/v3/tests/test_environmentvariables.py b/readthedocs/api/v3/tests/test_environmentvariables.py index e53053e0d42..21a6a075a90 100644 --- a/readthedocs/api/v3/tests/test_environmentvariables.py +++ b/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." + ] diff --git a/readthedocs/projects/migrations/0131_increase_env_var_size.py b/readthedocs/projects/migrations/0131_increase_env_var_size.py new file mode 100644 index 00000000000..3d6312b1bcb --- /dev/null +++ b/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 + ), + ), + ] diff --git a/readthedocs/projects/models.py b/readthedocs/projects/models.py index 66371a96845..72c8c080dd0 100644 --- a/readthedocs/projects/models.py +++ b/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() diff --git a/readthedocs/projects/validators.py b/readthedocs/projects/validators.py index ac2160995ed..174c4792711 100644 --- a/readthedocs/projects/validators.py +++ b/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." + ) + ) diff --git a/readthedocs/rtd_tests/tests/test_project_forms.py b/readthedocs/rtd_tests/tests/test_project_forms.py index aaa518ca33e..0e738fea171 100644 --- a/readthedocs/rtd_tests/tests/test_project_forms.py +++ b/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):