fix(versioning): reject legacy feature-state writes on v2 environments#7488
Merged
Conversation
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}/]
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
for more information, see https://pre-commit.ci
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7488 +/- ##
========================================
Coverage 98.44% 98.44%
========================================
Files 1398 1399 +1
Lines 52646 52764 +118
========================================
+ Hits 51826 51944 +118
Misses 820 820 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
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.
for more information, see https://pre-commit.ci
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.
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.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
Contributor
Docker builds report
|
Contributor
Playwright Test Results (oss - depot-ubuntu-latest-16)
Details
Playwright Test Results (oss - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-16)
Details
|
Contributor
Visual Regression16 screenshots compared. See report for details. |
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
Closes #7487
The non-versioning feature-state endpoints mutated the live
FeatureStaterow in place on v2-versioned environments — bypassing the version graph (no newEnvironmentFeatureVersion, no audit row, noNEW_VERSION_PUBLISHEDwebhook) and silently losing the change on rollback.This PR rejects
POST/PUT/PATCH/DELETEagainst those endpoints withHTTP 400on v2 environments and points callers at the versioning endpoint. Identity overrides remain allowed (they are not part of the version graph in v2).Blocked routes (on v2 envs unless identity-scoped):
POST/PUT/PATCH/DELETE /api/v1/environments/{api_key}/featurestates/[{id}/]POST/PUT/PATCH /api/v1/features/featurestates/[{id}/]How did you test this code?
8 new unit tests in
api/tests/unit/features/test_unit_features_views.pycover:POST/PUTon both endpoints → 400 on v2DELETEon the nested endpoint → 400 on v2 (row still exists)Each blocking test is parametrised over
admin_clientandadmin_master_api_key_client. mypy clean on the touched files.