Skip to content

Conversation

@rohan-chaturvedi
Copy link
Member

@rohan-chaturvedi rohan-chaturvedi commented Nov 8, 2025

🔍 Overview

  • Add custom validation rules to the graphql server to mitigate DoS via alias overloading and field duplications
  • Added server-side GraphQL validation to prevent users from being able to change their own roles
  • Added server-side GraphQL validation to the roles assigned to user invites
  • Added environment access validation when bulk processing secret mutations

Note

Adds GraphQL validation, centralizes validated client IP extraction, and tightens access controls for invites, roles, service accounts, and secret/environment mutations.

  • GraphQL:
    • Validation rules: Add DuplicateFieldLimitRule and AliasUsageLimitRule in backend/graphene/validation.py and enable via PrivateGraphQLView.validation_rules.
    • Middleware: Use central get_client_ip in backend/graphene/middleware.py.
  • Access Controls:
    • Organisation: Restrict invite roles to developer/service; block changing one’s own role in UpdateOrganisationMemberRole.
    • Service Accounts: Disallow roles with global access; ensure roles are organisation-scoped in create/update mutations.
    • Secrets/Environments: Add user_can_access_environment checks to bulk/individual secret mutations and related folder ops.
  • Networking:
    • Introduce api.utils.access.ip.get_client_ip (validated HTTP_X_REAL_IP) and replace ad-hoc implementations across emails.py, utils/rest.py, utils/access/middleware.py, views/kms.py, Graphene middleware, and queries/access.py.
  • Dependencies:
    • Bump graphene-django to 3.2.0 in backend/requirements.txt.

Written by Cursor Bugbot for commit da65c38. Configure here.

…s usage for graphene

Signed-off-by: rohan <rohan.chaturvedi@protonmail.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR upgrades graphene-django from version 3.0.0 to 3.2.0 and introduces custom GraphQL validation rules to protect against potential DoS attacks through excessive duplicate fields and alias usage.

Key changes:

  • Updated graphene-django dependency to version 3.2.0
  • Added two custom validation rules: DuplicateFieldLimitRule and AliasUsageLimitRule with configurable limits (default: 50 each)
  • Integrated the custom validation rules into the PrivateGraphQLView alongside the standard GraphQL validation rules

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
backend/requirements.txt Upgraded graphene-django from 3.0.0 to 3.2.0
backend/backend/graphene/validation.py Introduced new validation rules to limit duplicate field requests and alias usage in GraphQL queries
backend/api/views/graphql.py Integrated custom validation rules into the PrivateGraphQLView class

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Signed-off-by: rohan <rohan.chaturvedi@protonmail.com>
@rohan-chaturvedi rohan-chaturvedi marked this pull request as ready for review November 8, 2025 13:28
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@nimish-ks
Copy link
Member

@cursor review

Signed-off-by: rohan <rohan.chaturvedi@protonmail.com>
Signed-off-by: rohan <rohan.chaturvedi@protonmail.com>
@rohan-chaturvedi rohan-chaturvedi changed the title feat: implement custom validation rules for duplicate fields and alias usage for graphene feat: add misc graphql validation rules Nov 9, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@nimish-ks
Copy link
Member

@cursor review

nimish-ks and others added 3 commits November 10, 2025 18:56
* refactor: use standardized get_client_ip util

Signed-off-by: rohan <rohan.chaturvedi@protonmail.com>

* refactor: simplify IP retrieval in IsIPAllowed permission class by using get_client_ip utility

* refactor: streamline client IP retrieval in IPWhitelistMiddleware by utilizing get_client_ip utility

* refactor: enhance get_client_ip utility to validate IP addresses and improve retrieval logic

* debug: add print statement to log raw IP address in get_client_ip utility

* chore: add a blank line at the end of ip.py for consistency

* refactor: update client IP retrieval logic in get_client_ip utility to prioritize HTTP_X_REAL_IP

* refactor: remove debug print statement from get_client_ip utility

* fix: remove unnecessary fallbacks

Signed-off-by: rohan <rohan.chaturvedi@protonmail.com>

---------

Signed-off-by: rohan <rohan.chaturvedi@protonmail.com>
Co-authored-by: rohan <rohan.chaturvedi@protonmail.com>
Signed-off-by: rohan <rohan.chaturvedi@protonmail.com>
Signed-off-by: rohan <rohan.chaturvedi@protonmail.com>
@nimish-ks
Copy link
Member

@cursor review

@nimish-ks nimish-ks merged commit fe603d2 into main Nov 13, 2025
7 checks passed
@nimish-ks nimish-ks deleted the fix--graphql-server-hardening branch November 13, 2025 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants