From 91c3c8065609c9897125ab3f259b4a1e0d848a71 Mon Sep 17 00:00:00 2001 From: Gagan Trivedi Date: Tue, 12 May 2026 12:54:04 +0530 Subject: [PATCH 01/10] fix(versioning): reject legacy feature-state writes on v2 environments Direct writes through the legacy feature-state CRUD endpoints mutated the live FeatureState row in place on v2-versioned environments, bypassing the version graph: no new EnvironmentFeatureVersion, no audit row, no NEW_VERSION_PUBLISHED webhook, and the change was lost on rollback to a prior version. Reject these calls with 400 and point callers at the versioning endpoint. Identity overrides stay allowed since they are not part of the version graph. Affected endpoints: - POST/PUT/PATCH /api/v1/environments/{api_key}/featurestates/[{id}/] - POST/PUT/PATCH /api/v1/features/featurestates/[{id}/] --- api/features/exceptions.py | 17 ++ api/features/views.py | 50 +++++ .../unit/features/test_unit_features_views.py | 179 ++++++++++++++++++ 3 files changed, 246 insertions(+) diff --git a/api/features/exceptions.py b/api/features/exceptions.py index 52b2cef2321b..0541943799d1 100644 --- a/api/features/exceptions.py +++ b/api/features/exceptions.py @@ -13,3 +13,20 @@ def __init__(self, version: int): super(FeatureStateVersionAlreadyExistsError, self).__init__( f"Version {version} already exists for FeatureState." ) + + +class FeatureStateV2VersioningRequiredError(APIException): + status_code = status.HTTP_400_BAD_REQUEST + default_code = "v2_feature_versioning_required" + + def __init__( + self, environment_api_key: str, feature_id: int | None = None + ) -> None: + feature_path = str(feature_id) if feature_id is not None else "" + super().__init__( + detail=( + "This environment uses v2 feature versioning. Create a new " + "environment feature version via POST /api/v1/environments/" + f"{environment_api_key}/features/{feature_path}/versions/." + ) + ) diff --git a/api/features/views.py b/api/features/views.py index 965f01bdf79b..64b859620f8e 100644 --- a/api/features/views.py +++ b/api/features/views.py @@ -71,6 +71,7 @@ from webhooks.webhooks import WebhookEventType from .constants import INTERSECTION, UNION +from .exceptions import FeatureStateV2VersioningRequiredError from .features_service import get_overrides_data from .models import Feature, FeatureSegment, FeatureState from .multivariate.serializers import ( @@ -629,6 +630,33 @@ def _filter_queryset( return queryset +def _ensure_legacy_write_allowed( + environment: Environment, + *, + is_identity_override: bool, + feature_id: int | None = None, +) -> None: + # Direct writes against an environment or segment-override FeatureState on a + # v2 environment bypass the version graph and are silently lost on rollback. + # Identity overrides are not part of the version graph, so allow them. + if is_identity_override or not environment.use_v2_feature_versioning: + return + raise FeatureStateV2VersioningRequiredError( + environment_api_key=environment.api_key, + feature_id=feature_id, + ) + + +def _ensure_feature_state_legacy_write_allowed( + feature_state: FeatureState, +) -> None: + _ensure_legacy_write_allowed( + environment=feature_state.environment, # type: ignore[arg-type] + is_identity_override=feature_state.identity_id is not None, + feature_id=feature_state.feature_id, + ) + + @method_decorator( name="list", decorator=extend_schema( @@ -740,6 +768,12 @@ def create(self, request, *args, **kwargs): # type: ignore[no-untyped-def] if identity_pk: data["identity"] = identity_pk + _ensure_legacy_write_allowed( + environment=environment, + is_identity_override=bool(identity_pk), + feature_id=data.get("feature"), + ) + serializer = self.get_serializer(data=data) if serializer.is_valid(raise_exception=True): @@ -763,6 +797,7 @@ def update(self, request, *args, **kwargs): # type: ignore[no-untyped-def] feature state value. """ feature_state_to_update = self.get_object() + _ensure_feature_state_legacy_write_allowed(feature_state_to_update) feature_state_data = request.data # Check if feature state value was provided with request data. If so, create / update @@ -930,6 +965,21 @@ class SimpleFeatureStateViewSet( permission_classes = [FeatureStatePermissions] filterset_fields = ["environment", "feature", "feature_segment"] + def create(self, request, *args, **kwargs): # type: ignore[no-untyped-def] + environment_id = request.data.get("environment") + if environment_id: + environment = get_object_or_404(Environment, id=environment_id) + _ensure_legacy_write_allowed( + environment=environment, + is_identity_override=bool(request.data.get("identity")), + feature_id=request.data.get("feature"), + ) + return super().create(request, *args, **kwargs) + + def update(self, request, *args, **kwargs): # type: ignore[no-untyped-def] + _ensure_feature_state_legacy_write_allowed(self.get_object()) + return super().update(request, *args, **kwargs) + def get_queryset(self): # type: ignore[no-untyped-def] if getattr(self, "swagger_fake_view", False): return FeatureState.objects.none() diff --git a/api/tests/unit/features/test_unit_features_views.py b/api/tests/unit/features/test_unit_features_views.py index 042735aea104..198ce006f0d9 100644 --- a/api/tests/unit/features/test_unit_features_views.py +++ b/api/tests/unit/features/test_unit_features_views.py @@ -2856,6 +2856,185 @@ def test_update_feature_state__no_fsv_history__returns_200( assert response.status_code == status.HTTP_200_OK +def test_update_feature_state__v2_versioning_env__simple_endpoint__returns_400( + admin_client_new: APIClient, + environment_v2_versioning: Environment, + feature: Feature, +) -> None: + # Given + live_fs = FeatureState.objects.get( + environment=environment_v2_versioning, + feature=feature, + identity=None, + feature_segment=None, + ) + url = reverse("api-v1:features:featurestates-detail", args=[live_fs.id]) + data = { + "id": live_fs.id, + "enabled": True, + "feature_state_value": {"type": "unicode", "string_value": "should-be-blocked"}, + "environment": environment_v2_versioning.id, + "feature": feature.id, + } + + # When + response = admin_client_new.put( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + detail = response.json()["detail"] + assert "v2 feature versioning" in detail + assert environment_v2_versioning.api_key in detail + assert f"/features/{feature.id}/versions/" in detail + + +def test_update_feature_state__v2_versioning_env__nested_endpoint__returns_400( + admin_client_new: APIClient, + environment_v2_versioning: Environment, + feature: Feature, +) -> None: + # Given + live_fs = FeatureState.objects.get( + environment=environment_v2_versioning, + feature=feature, + identity=None, + feature_segment=None, + ) + url = reverse( + "api-v1:environments:environment-featurestates-detail", + args=[environment_v2_versioning.api_key, live_fs.id], + ) + data = { + "id": live_fs.id, + "enabled": True, + "feature_state_value": "should-be-blocked", + "environment": environment_v2_versioning.id, + "feature": feature.id, + "identity": None, + "feature_segment": None, + } + + # When + response = admin_client_new.put( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + + +def test_update_feature_state__v2_versioning_env__identity_override__allowed( + admin_client_new: APIClient, + environment_v2_versioning: Environment, + feature: Feature, + identity: Identity, +) -> None: + # Given - identity overrides live outside the version graph in v2 + identity_fs = FeatureState.objects.create( + feature=feature, + environment=environment_v2_versioning, + identity=identity, + enabled=False, + ) + url = reverse("api-v1:features:featurestates-detail", args=[identity_fs.id]) + data = { + "id": identity_fs.id, + "enabled": True, + "feature_state_value": {"type": "unicode", "string_value": "identity-override"}, + "environment": environment_v2_versioning.id, + "feature": feature.id, + "identity": identity.id, + } + + # When + response = admin_client_new.put( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_200_OK + + +def test_create_feature_state__v2_versioning_env__simple_endpoint__returns_400( + admin_client_new: APIClient, + environment_v2_versioning: Environment, + feature: Feature, +) -> None: + # Given + url = reverse("api-v1:features:featurestates-list") + data = { + "enabled": True, + "feature_state_value": {"type": "unicode", "string_value": "blocked"}, + "environment": environment_v2_versioning.id, + "feature": feature.id, + } + + # When + response = admin_client_new.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "v2 feature versioning" in response.json()["detail"] + + +def test_create_feature_state__v2_versioning_env__nested_endpoint__returns_400( + admin_client_new: APIClient, + environment_v2_versioning: Environment, + feature: Feature, + project: Project, +) -> None: + # Given - create a feature_segment so the create has somewhere to go + segment = Segment.objects.create(name="new_override_segment", project=project) + url = reverse( + "api-v1:environments:environment-featurestates-list", + args=[environment_v2_versioning.api_key], + ) + data = { + "enabled": True, + "feature_state_value": "blocked", + "feature": feature.id, + "feature_segment": {"segment": segment.id, "priority": 1}, + } + + # When + response = admin_client_new.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + + +def test_create_feature_state__v2_versioning_env__identity_override__allowed( + admin_client_new: APIClient, + environment_v2_versioning: Environment, + feature: Feature, + identity: Identity, +) -> None: + # Given - identity FS creates are not part of the version graph + url = reverse( + "api-v1:environments:identity-featurestates-list", + args=[environment_v2_versioning.api_key, identity.id], + ) + data = { + "enabled": True, + "feature_state_value": "identity-create", + "feature": feature.id, + } + + # When + response = admin_client_new.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_201_CREATED + + def test_update_feature_state__change_feature__returns_400( admin_client_new: APIClient, environment: Environment, From b771002cf09f7631dd3245c5d22abe6baf6de90a Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 12 May 2026 07:26:35 +0000 Subject: [PATCH 02/10] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- api/features/exceptions.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/api/features/exceptions.py b/api/features/exceptions.py index 0541943799d1..2390f9042bc8 100644 --- a/api/features/exceptions.py +++ b/api/features/exceptions.py @@ -19,9 +19,7 @@ class FeatureStateV2VersioningRequiredError(APIException): status_code = status.HTTP_400_BAD_REQUEST default_code = "v2_feature_versioning_required" - def __init__( - self, environment_api_key: str, feature_id: int | None = None - ) -> None: + def __init__(self, environment_api_key: str, feature_id: int | None = None) -> None: feature_path = str(feature_id) if feature_id is not None else "" super().__init__( detail=( From bea94d9f7a4bd58882c7a47ca303a8e81cbbc6b2 Mon Sep 17 00:00:00 2001 From: Gagan Trivedi Date: Tue, 12 May 2026 13:08:03 +0530 Subject: [PATCH 03/10] fix(versioning): block legacy feature-state DELETE on v2 environments DELETE on the nested feature-state endpoint had the same version-graph bypass as POST/PUT/PATCH: removing the live FS for an env or segment override silently mutated the live EnvironmentFeatureVersion's row set. Reject these calls with 400 too; identity-override deletes stay allowed. --- api/features/views.py | 4 ++ .../unit/features/test_unit_features_views.py | 51 +++++++++++++++++++ 2 files changed, 55 insertions(+) diff --git a/api/features/views.py b/api/features/views.py index 64b859620f8e..ad916b626434 100644 --- a/api/features/views.py +++ b/api/features/views.py @@ -832,6 +832,10 @@ def partial_update(self, request, *args, **kwargs): # type: ignore[no-untyped-d """ return self.update(request, *args, **kwargs) # type: ignore[no-untyped-call] + def destroy(self, request, *args, **kwargs): # type: ignore[no-untyped-def] + _ensure_feature_state_legacy_write_allowed(self.get_object()) + return super().destroy(request, *args, **kwargs) + def update_feature_state_value(self, value, feature_state): # type: ignore[no-untyped-def] feature_state_value_dict = feature_state.generate_feature_state_value_data( value diff --git a/api/tests/unit/features/test_unit_features_views.py b/api/tests/unit/features/test_unit_features_views.py index 198ce006f0d9..fdb7818e04c3 100644 --- a/api/tests/unit/features/test_unit_features_views.py +++ b/api/tests/unit/features/test_unit_features_views.py @@ -3035,6 +3035,57 @@ def test_create_feature_state__v2_versioning_env__identity_override__allowed( assert response.status_code == status.HTTP_201_CREATED +def test_delete_feature_state__v2_versioning_env__nested_endpoint__returns_400( + admin_client_new: APIClient, + environment_v2_versioning: Environment, + feature: Feature, +) -> None: + # Given - the live env-default FS attached to the current EFV + live_fs = FeatureState.objects.get( + environment=environment_v2_versioning, + feature=feature, + identity=None, + feature_segment=None, + ) + url = reverse( + "api-v1:environments:environment-featurestates-detail", + args=[environment_v2_versioning.api_key, live_fs.id], + ) + + # When + response = admin_client_new.delete(url) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert FeatureState.objects.filter(id=live_fs.id).exists() + + +def test_delete_feature_state__v2_versioning_env__identity_override__allowed( + admin_client_new: APIClient, + environment_v2_versioning: Environment, + feature: Feature, + identity: Identity, +) -> None: + # Given - identity overrides live outside the version graph + identity_fs = FeatureState.objects.create( + feature=feature, + environment=environment_v2_versioning, + identity=identity, + enabled=False, + ) + url = reverse( + "api-v1:environments:identity-featurestates-detail", + args=[environment_v2_versioning.api_key, identity.id, identity_fs.id], + ) + + # When + response = admin_client_new.delete(url) + + # Then + assert response.status_code == status.HTTP_204_NO_CONTENT + assert not FeatureState.objects.filter(id=identity_fs.id).exists() + + def test_update_feature_state__change_feature__returns_400( admin_client_new: APIClient, environment: Environment, From 446508003f6dbe19ec1df040d698e15922b4bc1a Mon Sep 17 00:00:00 2001 From: Gagan Trivedi Date: Tue, 12 May 2026 13:36:33 +0530 Subject: [PATCH 04/10] refactor(versioning): move direct-write guard into versioning service Co-locate the guard with update_flag() and the other v2-versioning policy in features/versioning/versioning_service.py, and move the exception alongside the other versioning errors. Rename the helpers to require_direct_state_write/_for_state to signal that they raise. Trim the exception message to a static one-liner. --- api/features/exceptions.py | 13 ------ api/features/versioning/exceptions.py | 6 +++ api/features/versioning/versioning_service.py | 19 +++++++++ api/features/views.py | 42 ++++--------------- .../unit/features/test_unit_features_views.py | 5 +-- 5 files changed, 33 insertions(+), 52 deletions(-) diff --git a/api/features/exceptions.py b/api/features/exceptions.py index 2390f9042bc8..31a83243d218 100644 --- a/api/features/exceptions.py +++ b/api/features/exceptions.py @@ -15,16 +15,3 @@ def __init__(self, version: int): ) -class FeatureStateV2VersioningRequiredError(APIException): - status_code = status.HTTP_400_BAD_REQUEST - default_code = "v2_feature_versioning_required" - - def __init__(self, environment_api_key: str, feature_id: int | None = None) -> None: - feature_path = str(feature_id) if feature_id is not None else "" - super().__init__( - detail=( - "This environment uses v2 feature versioning. Create a new " - "environment feature version via POST /api/v1/environments/" - f"{environment_api_key}/features/{feature_path}/versions/." - ) - ) diff --git a/api/features/versioning/exceptions.py b/api/features/versioning/exceptions.py index 3fb5817a2748..5d34b9d2f6f9 100644 --- a/api/features/versioning/exceptions.py +++ b/api/features/versioning/exceptions.py @@ -12,3 +12,9 @@ class FeatureVersionDeleteError(FeatureVersioningError): class CannotModifyLiveVersionError(FeatureVersioningError): status_code = status.HTTP_400_BAD_REQUEST + + +class DirectFeatureStateWriteNotAllowedError(FeatureVersioningError): + status_code = status.HTTP_400_BAD_REQUEST + default_code = "direct_feature_state_write_not_allowed" + default_detail = "This environment uses v2 feature versioning. Use the environment feature version endpoint instead." diff --git a/api/features/versioning/versioning_service.py b/api/features/versioning/versioning_service.py index e027692d384c..d5b6d171e1c2 100644 --- a/api/features/versioning/versioning_service.py +++ b/api/features/versioning/versioning_service.py @@ -13,9 +13,28 @@ FlagChangeSet, FlagChangeSetV2, ) +from features.versioning.exceptions import DirectFeatureStateWriteNotAllowedError from features.versioning.models import EnvironmentFeatureVersion +def require_direct_state_write( + environment: Environment, *, is_identity_override: bool +) -> None: + # Direct writes against an environment or segment-override FeatureState on a + # v2 environment bypass the version graph and are silently lost on rollback. + # Identity overrides are not part of the version graph, so allow them. + if is_identity_override or not environment.use_v2_feature_versioning: + return + raise DirectFeatureStateWriteNotAllowedError() + + +def require_direct_state_write_for_state(feature_state: FeatureState) -> None: + require_direct_state_write( + environment=feature_state.environment, # type: ignore[arg-type] + is_identity_override=feature_state.identity_id is not None, + ) + + def get_environment_flags_queryset( environment: Environment, feature_name: str = None, # type: ignore[assignment] diff --git a/api/features/views.py b/api/features/views.py index ad916b626434..e77c25a337af 100644 --- a/api/features/views.py +++ b/api/features/views.py @@ -71,7 +71,6 @@ from webhooks.webhooks import WebhookEventType from .constants import INTERSECTION, UNION -from .exceptions import FeatureStateV2VersioningRequiredError from .features_service import get_overrides_data from .models import Feature, FeatureSegment, FeatureState from .multivariate.serializers import ( @@ -107,6 +106,8 @@ ) from .tasks import trigger_feature_state_change_webhooks from .versioning.versioning_service import ( + require_direct_state_write, + require_direct_state_write_for_state, get_environment_flags_list, get_environment_flags_queryset, ) @@ -630,33 +631,6 @@ def _filter_queryset( return queryset -def _ensure_legacy_write_allowed( - environment: Environment, - *, - is_identity_override: bool, - feature_id: int | None = None, -) -> None: - # Direct writes against an environment or segment-override FeatureState on a - # v2 environment bypass the version graph and are silently lost on rollback. - # Identity overrides are not part of the version graph, so allow them. - if is_identity_override or not environment.use_v2_feature_versioning: - return - raise FeatureStateV2VersioningRequiredError( - environment_api_key=environment.api_key, - feature_id=feature_id, - ) - - -def _ensure_feature_state_legacy_write_allowed( - feature_state: FeatureState, -) -> None: - _ensure_legacy_write_allowed( - environment=feature_state.environment, # type: ignore[arg-type] - is_identity_override=feature_state.identity_id is not None, - feature_id=feature_state.feature_id, - ) - - @method_decorator( name="list", decorator=extend_schema( @@ -768,10 +742,9 @@ def create(self, request, *args, **kwargs): # type: ignore[no-untyped-def] if identity_pk: data["identity"] = identity_pk - _ensure_legacy_write_allowed( + require_direct_state_write( environment=environment, is_identity_override=bool(identity_pk), - feature_id=data.get("feature"), ) serializer = self.get_serializer(data=data) @@ -797,7 +770,7 @@ def update(self, request, *args, **kwargs): # type: ignore[no-untyped-def] feature state value. """ feature_state_to_update = self.get_object() - _ensure_feature_state_legacy_write_allowed(feature_state_to_update) + require_direct_state_write_for_state(feature_state_to_update) feature_state_data = request.data # Check if feature state value was provided with request data. If so, create / update @@ -833,7 +806,7 @@ def partial_update(self, request, *args, **kwargs): # type: ignore[no-untyped-d return self.update(request, *args, **kwargs) # type: ignore[no-untyped-call] def destroy(self, request, *args, **kwargs): # type: ignore[no-untyped-def] - _ensure_feature_state_legacy_write_allowed(self.get_object()) + require_direct_state_write_for_state(self.get_object()) return super().destroy(request, *args, **kwargs) def update_feature_state_value(self, value, feature_state): # type: ignore[no-untyped-def] @@ -973,15 +946,14 @@ def create(self, request, *args, **kwargs): # type: ignore[no-untyped-def] environment_id = request.data.get("environment") if environment_id: environment = get_object_or_404(Environment, id=environment_id) - _ensure_legacy_write_allowed( + require_direct_state_write( environment=environment, is_identity_override=bool(request.data.get("identity")), - feature_id=request.data.get("feature"), ) return super().create(request, *args, **kwargs) def update(self, request, *args, **kwargs): # type: ignore[no-untyped-def] - _ensure_feature_state_legacy_write_allowed(self.get_object()) + require_direct_state_write_for_state(self.get_object()) return super().update(request, *args, **kwargs) def get_queryset(self): # type: ignore[no-untyped-def] diff --git a/api/tests/unit/features/test_unit_features_views.py b/api/tests/unit/features/test_unit_features_views.py index fdb7818e04c3..4304874c9ce4 100644 --- a/api/tests/unit/features/test_unit_features_views.py +++ b/api/tests/unit/features/test_unit_features_views.py @@ -2884,10 +2884,7 @@ def test_update_feature_state__v2_versioning_env__simple_endpoint__returns_400( # Then assert response.status_code == status.HTTP_400_BAD_REQUEST - detail = response.json()["detail"] - assert "v2 feature versioning" in detail - assert environment_v2_versioning.api_key in detail - assert f"/features/{feature.id}/versions/" in detail + assert "v2 feature versioning" in response.json()["detail"] def test_update_feature_state__v2_versioning_env__nested_endpoint__returns_400( From 15ef12eb5e8879041877189219779468bf3db0a5 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 12 May 2026 08:09:34 +0000 Subject: [PATCH 05/10] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- api/features/exceptions.py | 2 -- api/features/views.py | 4 ++-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/api/features/exceptions.py b/api/features/exceptions.py index 31a83243d218..52b2cef2321b 100644 --- a/api/features/exceptions.py +++ b/api/features/exceptions.py @@ -13,5 +13,3 @@ def __init__(self, version: int): super(FeatureStateVersionAlreadyExistsError, self).__init__( f"Version {version} already exists for FeatureState." ) - - diff --git a/api/features/views.py b/api/features/views.py index e77c25a337af..35bb03e18c9d 100644 --- a/api/features/views.py +++ b/api/features/views.py @@ -106,10 +106,10 @@ ) from .tasks import trigger_feature_state_change_webhooks from .versioning.versioning_service import ( - require_direct_state_write, - require_direct_state_write_for_state, get_environment_flags_list, get_environment_flags_queryset, + require_direct_state_write, + require_direct_state_write_for_state, ) logger = logging.getLogger(__name__) From 56831a1beba16a29ca256db96307980cc6f9f52f Mon Sep 17 00:00:00 2001 From: Gagan Trivedi Date: Tue, 12 May 2026 13:45:10 +0530 Subject: [PATCH 06/10] test(versioning): assert detail in all direct-write 400 responses Drop the orienting comment from require_direct_state_write; the function name and exception class carry the meaning. Add the "v2 feature versioning" detail assertion to the three blocking tests that were missing it for consistency. --- api/features/versioning/versioning_service.py | 3 --- api/tests/unit/features/test_unit_features_views.py | 3 +++ 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api/features/versioning/versioning_service.py b/api/features/versioning/versioning_service.py index d5b6d171e1c2..e3271e6fe8b6 100644 --- a/api/features/versioning/versioning_service.py +++ b/api/features/versioning/versioning_service.py @@ -20,9 +20,6 @@ def require_direct_state_write( environment: Environment, *, is_identity_override: bool ) -> None: - # Direct writes against an environment or segment-override FeatureState on a - # v2 environment bypass the version graph and are silently lost on rollback. - # Identity overrides are not part of the version graph, so allow them. if is_identity_override or not environment.use_v2_feature_versioning: return raise DirectFeatureStateWriteNotAllowedError() diff --git a/api/tests/unit/features/test_unit_features_views.py b/api/tests/unit/features/test_unit_features_views.py index 4304874c9ce4..9743a9015af5 100644 --- a/api/tests/unit/features/test_unit_features_views.py +++ b/api/tests/unit/features/test_unit_features_views.py @@ -2920,6 +2920,7 @@ def test_update_feature_state__v2_versioning_env__nested_endpoint__returns_400( # Then assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "v2 feature versioning" in response.json()["detail"] def test_update_feature_state__v2_versioning_env__identity_override__allowed( @@ -3004,6 +3005,7 @@ def test_create_feature_state__v2_versioning_env__nested_endpoint__returns_400( # Then assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "v2 feature versioning" in response.json()["detail"] def test_create_feature_state__v2_versioning_env__identity_override__allowed( @@ -3054,6 +3056,7 @@ def test_delete_feature_state__v2_versioning_env__nested_endpoint__returns_400( # Then assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "v2 feature versioning" in response.json()["detail"] assert FeatureState.objects.filter(id=live_fs.id).exists() From 336be79613eab09310f5bbfdabf04684715cedcd Mon Sep 17 00:00:00 2001 From: Gagan Trivedi Date: Tue, 12 May 2026 13:48:39 +0530 Subject: [PATCH 07/10] test(versioning): rename direct-write tests to match FT003 template --- .../unit/features/test_unit_features_views.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/api/tests/unit/features/test_unit_features_views.py b/api/tests/unit/features/test_unit_features_views.py index 9743a9015af5..2f7b721c362d 100644 --- a/api/tests/unit/features/test_unit_features_views.py +++ b/api/tests/unit/features/test_unit_features_views.py @@ -2856,7 +2856,7 @@ def test_update_feature_state__no_fsv_history__returns_200( assert response.status_code == status.HTTP_200_OK -def test_update_feature_state__v2_versioning_env__simple_endpoint__returns_400( +def test_update_feature_state__v2_env_via_simple_endpoint__returns_400( admin_client_new: APIClient, environment_v2_versioning: Environment, feature: Feature, @@ -2887,7 +2887,7 @@ def test_update_feature_state__v2_versioning_env__simple_endpoint__returns_400( assert "v2 feature versioning" in response.json()["detail"] -def test_update_feature_state__v2_versioning_env__nested_endpoint__returns_400( +def test_update_feature_state__v2_env_via_nested_endpoint__returns_400( admin_client_new: APIClient, environment_v2_versioning: Environment, feature: Feature, @@ -2923,7 +2923,7 @@ def test_update_feature_state__v2_versioning_env__nested_endpoint__returns_400( assert "v2 feature versioning" in response.json()["detail"] -def test_update_feature_state__v2_versioning_env__identity_override__allowed( +def test_update_feature_state__v2_env_identity_override__returns_200( admin_client_new: APIClient, environment_v2_versioning: Environment, feature: Feature, @@ -2955,7 +2955,7 @@ def test_update_feature_state__v2_versioning_env__identity_override__allowed( assert response.status_code == status.HTTP_200_OK -def test_create_feature_state__v2_versioning_env__simple_endpoint__returns_400( +def test_create_feature_state__v2_env_via_simple_endpoint__returns_400( admin_client_new: APIClient, environment_v2_versioning: Environment, feature: Feature, @@ -2979,7 +2979,7 @@ def test_create_feature_state__v2_versioning_env__simple_endpoint__returns_400( assert "v2 feature versioning" in response.json()["detail"] -def test_create_feature_state__v2_versioning_env__nested_endpoint__returns_400( +def test_create_feature_state__v2_env_via_nested_endpoint__returns_400( admin_client_new: APIClient, environment_v2_versioning: Environment, feature: Feature, @@ -3008,7 +3008,7 @@ def test_create_feature_state__v2_versioning_env__nested_endpoint__returns_400( assert "v2 feature versioning" in response.json()["detail"] -def test_create_feature_state__v2_versioning_env__identity_override__allowed( +def test_create_feature_state__v2_env_identity_override__returns_201( admin_client_new: APIClient, environment_v2_versioning: Environment, feature: Feature, @@ -3034,7 +3034,7 @@ def test_create_feature_state__v2_versioning_env__identity_override__allowed( assert response.status_code == status.HTTP_201_CREATED -def test_delete_feature_state__v2_versioning_env__nested_endpoint__returns_400( +def test_delete_feature_state__v2_env_via_nested_endpoint__returns_400( admin_client_new: APIClient, environment_v2_versioning: Environment, feature: Feature, @@ -3060,7 +3060,7 @@ def test_delete_feature_state__v2_versioning_env__nested_endpoint__returns_400( assert FeatureState.objects.filter(id=live_fs.id).exists() -def test_delete_feature_state__v2_versioning_env__identity_override__allowed( +def test_delete_feature_state__v2_env_identity_override__returns_204( admin_client_new: APIClient, environment_v2_versioning: Environment, feature: Feature, From c595dd70abf51b435794f171a7de55aea55ad42e Mon Sep 17 00:00:00 2001 From: Gagan Trivedi Date: Tue, 12 May 2026 14:08:09 +0530 Subject: [PATCH 08/10] fix(versioning): allow direct-write when explicitly targeting a version The guard was too aggressive and rejected supported v2 flows where the caller participates in the version graph explicitly. Two refinements: - POST on the simple endpoint with environment_feature_version in the body is the supported way to create a segment override on a draft EFV. Skip the guard in that case. - PUT/PATCH/DELETE against a FeatureState attached to an unpublished (draft) EnvironmentFeatureVersion is modifying a draft, not bypassing the version graph. Skip the guard in that case too. Unblocks test_4eyes_workflow_with_v2_versioning. Adds two unit tests covering both new allow paths. --- api/features/versioning/versioning_service.py | 5 ++ api/features/views.py | 3 +- .../unit/features/test_unit_features_views.py | 69 +++++++++++++++++++ 3 files changed, 76 insertions(+), 1 deletion(-) diff --git a/api/features/versioning/versioning_service.py b/api/features/versioning/versioning_service.py index e3271e6fe8b6..531cc9eba6d9 100644 --- a/api/features/versioning/versioning_service.py +++ b/api/features/versioning/versioning_service.py @@ -26,6 +26,11 @@ def require_direct_state_write( def require_direct_state_write_for_state(feature_state: FeatureState) -> None: + # FS rows attached to an unpublished EFV are a draft, so direct mutation is + # part of the versioning flow rather than a bypass of it. + efv = feature_state.environment_feature_version + if efv is not None and not efv.published: + return require_direct_state_write( environment=feature_state.environment, # type: ignore[arg-type] is_identity_override=feature_state.identity_id is not None, diff --git a/api/features/views.py b/api/features/views.py index 35bb03e18c9d..5aec6fe8bee8 100644 --- a/api/features/views.py +++ b/api/features/views.py @@ -944,7 +944,8 @@ class SimpleFeatureStateViewSet( def create(self, request, *args, **kwargs): # type: ignore[no-untyped-def] environment_id = request.data.get("environment") - if environment_id: + targets_version = bool(request.data.get("environment_feature_version")) + if environment_id and not targets_version: environment = get_object_or_404(Environment, id=environment_id) require_direct_state_write( environment=environment, diff --git a/api/tests/unit/features/test_unit_features_views.py b/api/tests/unit/features/test_unit_features_views.py index 2f7b721c362d..dfc07904386b 100644 --- a/api/tests/unit/features/test_unit_features_views.py +++ b/api/tests/unit/features/test_unit_features_views.py @@ -3034,6 +3034,75 @@ def test_create_feature_state__v2_env_identity_override__returns_201( assert response.status_code == status.HTTP_201_CREATED +def test_create_feature_state__v2_env_targets_version__returns_201( + admin_client_new: APIClient, + environment_v2_versioning: Environment, + feature: Feature, + project: Project, +) -> None: + # Given - a draft EFV plus a feature_segment attached to it + draft_version = EnvironmentFeatureVersion.objects.create( + feature=feature, environment=environment_v2_versioning + ) + segment = Segment.objects.create(name="seg_targets_version", project=project) + feature_segment = FeatureSegment.objects.create( + feature=feature, + segment=segment, + environment=environment_v2_versioning, + environment_feature_version=draft_version, + priority=1, + ) + url = reverse("api-v1:features:featurestates-list") + data = { + "enabled": True, + "feature_state_value": {"type": "unicode", "string_value": "override"}, + "environment": environment_v2_versioning.id, + "feature": feature.id, + "feature_segment": feature_segment.id, + "environment_feature_version": str(draft_version.uuid), + } + + # When + response = admin_client_new.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_201_CREATED + + +def test_update_feature_state__v2_env_draft_version__returns_200( + admin_client_new: APIClient, + environment_v2_versioning: Environment, + feature: Feature, +) -> None: + # Given - a new draft EFV (post_save clones the previous version's FSes) + draft_version = EnvironmentFeatureVersion.objects.create( + feature=feature, environment=environment_v2_versioning + ) + draft_fs = FeatureState.objects.get( + environment_feature_version=draft_version, + identity=None, + feature_segment=None, + ) + url = reverse("api-v1:features:featurestates-detail", args=[draft_fs.id]) + data = { + "id": draft_fs.id, + "enabled": True, + "feature_state_value": {"type": "unicode", "string_value": "draft-update"}, + "environment": environment_v2_versioning.id, + "feature": feature.id, + } + + # When + response = admin_client_new.put( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_200_OK + + def test_delete_feature_state__v2_env_via_nested_endpoint__returns_400( admin_client_new: APIClient, environment_v2_versioning: Environment, From a41c6eb7ab0b3fbbf27c2ae45ec39a0e0b3e064a Mon Sep 17 00:00:00 2001 From: Gagan Trivedi Date: Tue, 12 May 2026 15:07:42 +0530 Subject: [PATCH 09/10] fix(versioning): reject create-segment-override on v2 environments The `POST /environments/{api_key}/features/{feature_pk}/create-segment-override/` view never threaded `environment_feature_version` into the serializer context, so on v2 environments the nested `CustomCreateSegmentOverrideFeatureSegmentSerializer` hit an `AssertionError` and returned HTTP 500. Reuse `require_direct_state_write` from the v2 direct-write guard to return a documented HTTP 400 instead, and convert the now-unreachable `assert` in the nested serializer to a `ValidationError` so any future v2 caller that forgets the context still gets a 400 rather than a 500. --- api/features/feature_segments/serializers.py | 7 ++-- api/features/views.py | 2 ++ .../unit/features/test_unit_features_views.py | 32 +++++++++++++++++++ 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/api/features/feature_segments/serializers.py b/api/features/feature_segments/serializers.py index c3b566005ba0..e4869ce345d2 100644 --- a/api/features/feature_segments/serializers.py +++ b/api/features/feature_segments/serializers.py @@ -60,8 +60,11 @@ def save(self, **kwargs: typing.Any) -> FeatureSegment: priority: int | None = self.validated_data.get("priority", None) - if kwargs["environment"].use_v2_feature_versioning: # pragma: no cover - assert kwargs["environment_feature_version"] is not None, ( + if ( + kwargs["environment"].use_v2_feature_versioning + and kwargs.get("environment_feature_version") is None + ): + raise serializers.ValidationError( "Must provide environment_feature_version for environment using v2 versioning" ) diff --git a/api/features/views.py b/api/features/views.py index 5aec6fe8bee8..345849738428 100644 --- a/api/features/views.py +++ b/api/features/views.py @@ -1193,6 +1193,8 @@ def create_segment_override( # type: ignore[no-untyped-def] environment = get_object_or_404(Environment, api_key=environment_api_key) feature = get_object_or_404(Feature, project=environment.project, pk=feature_pk) + require_direct_state_write(environment=environment, is_identity_override=False) + serializer = CustomCreateSegmentOverrideFeatureStateSerializer( data=request.data, context={"environment": environment, "feature": feature} ) diff --git a/api/tests/unit/features/test_unit_features_views.py b/api/tests/unit/features/test_unit_features_views.py index dfc07904386b..c524a8e8b010 100644 --- a/api/tests/unit/features/test_unit_features_views.py +++ b/api/tests/unit/features/test_unit_features_views.py @@ -1364,6 +1364,38 @@ def test_create_segment_override__admin_user__creates_override( assert created_override.get_feature_state_value() == string_value +def test_create_segment_override__v2_versioning_environment__returns_400( + admin_client_new: APIClient, + feature: Feature, + segment: Segment, + environment_v2_versioning: Environment, +) -> None: + # Given + url = reverse( + "api-v1:environments:create-segment-override", + args=[environment_v2_versioning.api_key, feature.id], + ) + data = { + "feature_state_value": {"string_value": "foo"}, + "enabled": True, + "feature_segment": {"segment": segment.id}, + } + + # When + response = admin_client_new.post( + url, data=json.dumps(data), content_type="application/json" + ) + + # Then + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert "v2 feature versioning" in response.json()["detail"] + assert not FeatureState.objects.filter( + feature=feature, + environment=environment_v2_versioning, + feature_segment__segment=segment, + ).exists() + + def test_get_flags__user_throttle_set__is_not_throttled( # type: ignore[no-untyped-def] api_client: APIClient, environment: Environment, From 8bf6c953d4bf93131e83a0e68c66dca9418ee419 Mon Sep 17 00:00:00 2001 From: Gagan Trivedi Date: Tue, 12 May 2026 15:19:02 +0530 Subject: [PATCH 10/10] refactor(versioning): drop unreachable v2 check in nested segment serializer With the view-layer guard in `create_segment_override` and the always-set context in `EnvironmentFeatureVersionFeatureStatesViewSet`, no caller can reach the nested serializer's save without `environment_feature_version` on a v2 environment. --- api/features/feature_segments/serializers.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/api/features/feature_segments/serializers.py b/api/features/feature_segments/serializers.py index e4869ce345d2..f435ffb844e2 100644 --- a/api/features/feature_segments/serializers.py +++ b/api/features/feature_segments/serializers.py @@ -60,14 +60,6 @@ def save(self, **kwargs: typing.Any) -> FeatureSegment: priority: int | None = self.validated_data.get("priority", None) - if ( - kwargs["environment"].use_v2_feature_versioning - and kwargs.get("environment_feature_version") is None - ): - raise serializers.ValidationError( - "Must provide environment_feature_version for environment using v2 versioning" - ) - if priority is not None: collision_qs = FeatureSegment.objects.filter( environment=kwargs["environment"],