Skip to content

Conversation

@gagantrivedi
Copy link
Member

@gagantrivedi gagantrivedi commented Dec 8, 2025

Thanks for submitting a PR! Please check the boxes below:

  • I have added information to docs/ if required so people know about the feature!
  • I have filled in the "Changes" section below?
  • I have filled in the "How did you test this code" section below?
  • I have used a Conventional Commit title for this Pull Request

Changes

Make the project field read-only during environment updates to prevent moving an environment to a different project

The vulnerability allowed an attacker with access to their own environment to modify the project field in the PUT request body, effectively moving their environment into a victim's project.

Fix: Override init in CreateUpdateEnvironmentSerializer to set project field as read-only when instance exists (update operation).

How did you test this code?

Adds unit test

@gagantrivedi gagantrivedi requested a review from a team as a code owner December 8, 2025 08:45
@gagantrivedi gagantrivedi requested review from khvn26 and removed request for a team December 8, 2025 08:45
@vercel
Copy link

vercel bot commented Dec 8, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

3 Skipped Deployments
Project Deployment Review Updated (UTC)
docs Ignored Ignored Preview Dec 25, 2025 8:36am
flagsmith-frontend-preview Ignored Ignored Preview Dec 25, 2025 8:36am
flagsmith-frontend-staging Ignored Ignored Preview Dec 25, 2025 8:36am

@github-actions github-actions bot added api Issue related to the REST API fix labels Dec 8, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Dec 8, 2025

Docker builds report

Image Build Status Security report
ghcr.io/flagsmith/flagsmith-e2e:pr-6384 Finished ✅ Skipped
ghcr.io/flagsmith/flagsmith-frontend:pr-6384 Finished ✅ Results
ghcr.io/flagsmith/flagsmith-api-test:pr-6384 Finished ✅ Skipped
ghcr.io/flagsmith/flagsmith:pr-6384 Finished ✅ Results
ghcr.io/flagsmith/flagsmith-api:pr-6384 Finished ✅ Results
ghcr.io/flagsmith/flagsmith-private-cloud:pr-6384 Finished ✅ Results

Comment on lines 133 to 137
def __init__(self, *args: Any, **kwargs: Any) -> None:
super().__init__(*args, **kwargs) # type: ignore[no-untyped-call]
# Prevent IDOR: project cannot be changed after creation
if self.instance is not None:
self.fields["project"].read_only = True
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A neater option here is probably just to separate the Create and Update serializers?

@codecov
Copy link

codecov bot commented Dec 8, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.07%. Comparing base (f7ca9cd) to head (28e10e7).
⚠️ Report is 36 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6384      +/-   ##
==========================================
+ Coverage   98.02%   98.07%   +0.04%     
==========================================
  Files        1282     1292      +10     
  Lines       45498    46526    +1028     
==========================================
+ Hits        44600    45629    +1029     
+ Misses        898      897       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@gagantrivedi gagantrivedi force-pushed the fix/idor-environment-project-validation branch from 90b1762 to cd0e004 Compare December 8, 2025 10:33
@github-actions github-actions bot added fix and removed fix labels Dec 8, 2025
Make the `project` field read-only during environment updates to prevent
attackers from moving an environment to a different project they don't own.

The vulnerability allowed an attacker with access to their own environment
to modify the `project` field in the PUT request body, effectively moving
their environment into a victim's project.

Fix: Override __init__ in CreateUpdateEnvironmentSerializer to set
project field as read-only when instance exists (update operation).
@gagantrivedi gagantrivedi force-pushed the fix/idor-environment-project-validation branch from cd0e004 to d29b554 Compare December 8, 2025 11:38
@github-actions github-actions bot added fix and removed fix labels Dec 8, 2025
matthewelwell
matthewelwell previously approved these changes Dec 17, 2025
Copy link
Contributor

@matthewelwell matthewelwell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving with a couple of minor comments.

Comment on lines 137 to 141
# handle `project` not being part of the data
# When request comes from yasg2(as part of schema generation)
project_id = view.request.data.get("project")
if not project_id:
return None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another (perhaps more descriptive) option here would be:

Suggested change
# handle `project` not being part of the data
# When request comes from yasg2(as part of schema generation)
project_id = view.request.data.get("project")
if not project_id:
return None
if getattr(self.request, "swagger_fake_view", False):
return Subscription.objects.none()
project_id = self.request.data["project"]

Comment on lines 157 to 158
if self.instance is None:
return None
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should never be the case, right? Is this to handle swagger again? If so, I prefer the explicit approach I suggested above.

Address PR review comments by using explicit swagger_fake_view attribute
check instead of implicit project_id presence check.
@gagantrivedi gagantrivedi force-pushed the fix/idor-environment-project-validation branch from 733c2ae to 28e10e7 Compare December 25, 2025 08:36
@github-actions github-actions bot added fix and removed fix labels Dec 25, 2025
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the final PR Bugbot will review for you during this billing cycle

Your free Bugbot reviews will reset on January 10

Details

Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api Issue related to the REST API fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants