fix(API): Nested viewsets expose data across projects and organisations#7945
fix(API): Nested viewsets expose data across projects and organisations#7945khvn26 wants to merge 4 commits into
Conversation
`MultivariateFeatureOptionViewSet.get_queryset` resolved the feature by bare `feature_pk`, ignoring `project_pk`. The list action runs no object-level permission check, so any user with a project could read another project's multivariate options by enumerating feature ids. Scope the feature lookup to the URL's project, mirroring `create`. beep boop
…ojects on list
`FeatureExternalResourceViewSet.get_queryset` filtered the list by bare
`feature_pk`, and `FeaturePermissions` applies no project check on the
list action ("handled by the view"). So any authenticated user could
read another feature's linked issue/PR resources by enumerating feature
ids.
Scope the feature lookup to the caller's permitted projects, matching
the pattern used by `FeatureViewSet`.
beep boop
… any authenticated user `UserPermissionGroupEnvironmentPermissionsViewSet` was gated only by `IsAuthenticated`, unlike its sibling `UserEnvironmentPermissionsViewSet` which also uses `NestedEnvironmentPermissions`. Since the environment client-side api key is not a secret, any authenticated user could read, and delete, another organisation's environment group permission assignments regardless of membership. Add `NestedEnvironmentPermissions` so the caller must hold the relevant permission on the environment. beep boop
`GithubRepositoryViewSet.get_queryset` filtered by bare `github_pk`, and `HasPermissionToGithubConfiguration` only checked membership of the URL's `organisation_pk`. Neither tied the GitHub configuration to that organisation, so a member of one organisation could read (and, via the create path, write) repositories under another organisation's configuration. Validate that the configuration in the URL belongs to the organisation in `HasPermissionToGithubConfiguration`, covering every action. A non-numeric `github_pk` is left for the view to reject with a 400. beep boop
|
Bugbot is not enabled for this team, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
📝 WalkthroughWalkthroughThis PR tightens authorisation checks across several endpoints to prevent cross-organisation and cross-project data access. It adds Estimated code review effort: 3 (Moderate) | ~25 minutes Changes
Sequence Diagram(s)See per-layer diagrams in the hidden review stack artifact above. Related issues: Not specified in the provided context. Related PRs: Not specified in the provided context. Suggested labels: security, bug, api Suggested reviewers: Not specified in the provided context. 🐰 A hop through permissions, tight and true, Comment |
Docker builds report
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #7945 +/- ##
=======================================
Coverage 98.63% 98.63%
=======================================
Files 1497 1497
Lines 59189 59240 +51
=======================================
+ Hits 58383 58434 +51
Misses 806 806 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 084d2baf-5008-457f-9618-316511df206d
📒 Files selected for processing (8)
api/environments/permissions/views.pyapi/features/feature_external_resources/views.pyapi/features/multivariate/views.pyapi/integrations/github/permissions.pyapi/tests/unit/environments/permissions/test_unit_environments_views.pyapi/tests/unit/features/multivariate/test_unit_multivariate_views.pyapi/tests/unit/features/test_unit_feature_external_resources_views.pyapi/tests/unit/integrations/github/test_unit_github_views.py
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-16)
Details
Playwright Test Results (private-cloud - depot-ubuntu-latest-arm-16)
Details
|
Visual Regression19 screenshots compared. See report for details. |
1 participant
Thanks for submitting a PR! Please check the boxes below:
docs/if required so people know about the feature.Changes
In this PR, we fix four vulnerabilites:
MultivariateFeatureOptionViewSetresolved the feature by barefeature_pk, so a user could read another project's multivariate options. Scoped the lookup to the URL's project, mirroringcreate.FeatureExternalResourceViewSetfiltered the list by barefeature_pk, andFeaturePermissionsapplied no project check onlist. Scoped the lookup to the caller's permitted projects, matchingFeatureViewSet.UserPermissionGroupEnvironmentPermissionsViewSetgated only byIsAuthenticated, unlike its siblingUserEnvironmentPermissionsViewSet. Because the environment client-side api key is not a secret, any authenticated user could read and delete another organisation's environment group permission assignments. Added the missingNestedEnvironmentPermissions.GithubRepositoryViewSetfiltered by baregithub_pkwhile the permission only checked membership of the URL'sorganisation_pk, neither tying the configuration to that organisation. Validated the configuration belongs to the organisation inHasPermissionToGithubConfiguration, covering every action.How did you test this code?
Added regression tests.