Codemod for disabling GraphQL introspection (#542)

* Initial implementation of disable-grapql-introspection * Refactored resolve methods and added tests * fixup! Refactored resolve methods and added tests * Bugfixes and tests * Sonar version of codemod * fixup! Sonar version of codemod * Refactoring * Apply suggestions from code review Co-authored-by: Dan D'Avella <[email protected]> --------- Co-authored-by: Dan D'Avella <[email protected]>
pixee · May 13, 2024 · 989cd69 · 989cd69
1 parent 17ed312
commit 989cd69
Show file tree

Hide file tree

Showing 16 changed files with 925 additions and 11 deletions.
diff --git a/integration_tests/sonar/test_sonar_disable_graphql_introspection.py b/integration_tests/sonar/test_sonar_disable_graphql_introspection.py
@@ -0,0 +1,63 @@
+from codemodder.codemods.test import SonarIntegrationTest
+from core_codemods.disable_graphql_introspection import (
+    DisableGraphQLIntrospectionTransform,
+)
+from core_codemods.sonar.sonar_disable_graphql_introspection import (
+    SonarDisableGraphQLIntrospection,
+)
+
+
+class TestSonarDisableGraphQLIntrospection(SonarIntegrationTest):
+    codemod = SonarDisableGraphQLIntrospection
+    code_path = "tests/samples/disable_graphql_introspection.py"
+    expected_new_code = """\
+    from graphql_server.flask import GraphQLView
+    from flask import Flask
+    from graphql import (
+        GraphQLSchema, GraphQLObjectType, GraphQLField, GraphQLString)
+    from graphql.validation import NoSchemaIntrospectionCustomRule
+
+    schema = GraphQLSchema(
+        query=GraphQLObjectType(
+            name='RootQueryType',
+            fields={
+                'hello': GraphQLField(
+                    GraphQLString,
+                    resolve=lambda obj, info: 'world')
+            }))
+
+    app = Flask(__name__)
+
+    app.add_url_rule("/api",
+        view_func=GraphQLView.as_view(
+            name="api",
+            schema=schema,
+        validation_rules = [NoSchemaIntrospectionCustomRule,]),
+    )
+    """
+
+    # fmt: off
+    expected_diff = (
+        """--- \n"""
+        """+++ \n"""
+        """@@ -2,6 +2,7 @@\n"""
+        """ from flask import Flask\n"""
+        """ from graphql import (\n"""
+        """     GraphQLSchema, GraphQLObjectType, GraphQLField, GraphQLString)\n"""
+        """+from graphql.validation import NoSchemaIntrospectionCustomRule\n"""
+        """ \n"""
+        """ schema = GraphQLSchema(\n"""
+        """     query=GraphQLObjectType(\n"""
+        """@@ -18,5 +19,5 @@\n"""
+        """     view_func=GraphQLView.as_view(\n"""
+        """         name="api",\n"""
+        """         schema=schema,\n"""
+        """-    ),\n"""
+        """+    validation_rules = [NoSchemaIntrospectionCustomRule,]),\n"""
+        """ )\n"""
+    )
+    # fmt: on
+
+    expected_line_change = "18"
+    change_description = DisableGraphQLIntrospectionTransform.change_description
+    num_changed_files = 1
diff --git a/integration_tests/test_disable_graphql_introspection.py b/integration_tests/test_disable_graphql_introspection.py
@@ -0,0 +1,84 @@
+from codemodder.codemods.test.integration_utils import BaseIntegrationTest
+from core_codemods.disable_graphql_introspection import (
+    DisableGraphQLIntrospection,
+    DisableGraphQLIntrospectionTransform,
+)
+
+
+class TestDisableGraphQLIntrospection(BaseIntegrationTest):
+    codemod = DisableGraphQLIntrospection
+    original_code = """
+        from graphql_server.flask import GraphQLView
+        from flask import Flask
+        from graphql import (
+            GraphQLSchema, GraphQLObjectType, GraphQLField, GraphQLString)
+
+        schema = GraphQLSchema(
+            query=GraphQLObjectType(
+                name='RootQueryType',
+                fields={
+                    'hello': GraphQLField(
+                        GraphQLString,
+                        resolve=lambda obj, info: 'world')
+                }))
+
+        app = Flask(__name__)
+
+        app.add_url_rule("/api",
+            view_func=GraphQLView.as_view(
+                name="api",
+                schema=schema,
+            ),
+        )
+    """
+    expected_new_code = """
+        from graphql_server.flask import GraphQLView
+        from flask import Flask
+        from graphql import (
+            GraphQLSchema, GraphQLObjectType, GraphQLField, GraphQLString)
+        from graphql.validation import NoSchemaIntrospectionCustomRule
+
+        schema = GraphQLSchema(
+            query=GraphQLObjectType(
+                name='RootQueryType',
+                fields={
+                    'hello': GraphQLField(
+                        GraphQLString,
+                        resolve=lambda obj, info: 'world')
+                }))
+
+        app = Flask(__name__)
+
+        app.add_url_rule("/api",
+            view_func=GraphQLView.as_view(
+                name="api",
+                schema=schema,
+            validation_rules = [NoSchemaIntrospectionCustomRule,]),
+        )
+    """
+
+    # fmt: off
+    expected_diff = (
+        """--- \n"""
+        """+++ \n"""
+        """@@ -2,6 +2,7 @@\n"""
+        """ from flask import Flask\n"""
+        """ from graphql import (\n"""
+        """     GraphQLSchema, GraphQLObjectType, GraphQLField, GraphQLString)\n"""
+        """+from graphql.validation import NoSchemaIntrospectionCustomRule\n"""
+        """ \n"""
+        """ schema = GraphQLSchema(\n"""
+        """     query=GraphQLObjectType(\n"""
+        """@@ -18,5 +19,5 @@\n"""
+        """     view_func=GraphQLView.as_view(\n"""
+        """         name="api",\n"""
+        """         schema=schema,\n"""
+        """-    ),\n"""
+        """+    validation_rules = [NoSchemaIntrospectionCustomRule,]),\n"""
+        """ )"""
+    )
+    # fmt: on
+
+    expected_line_change = "18"
+    change_description = DisableGraphQLIntrospectionTransform.change_description
+    num_changed_files = 1
diff --git a/pyproject.toml b/pyproject.toml
@@ -69,6 +69,7 @@ test = [
     "numpy~=1.26.0",
     "flask_wtf~=1.2.0",
     "fickling~=0.1.0,>=0.1.3",
+    "graphql-server~=3.0.0b7",
 ]
 complexity = [
     "radon==6.0.*",

diff --git a/src/codemodder/codemods/test/integration_utils.py b/src/codemodder/codemods/test/integration_utils.py
@@ -256,13 +256,21 @@ class SonarIntegrationTest(BaseIntegrationTest):
     @classmethod
     def setup_class(cls):
         cls.codemod_registry = registry.load_registered_codemods()
-        cls.original_code, cls.expected_new_code = original_and_expected_from_code_path(
-            cls.code_path, cls.replacement_lines
-        )
-
-        cls.code_filename = os.path.relpath(cls.code_path, SAMPLES_DIR)
-        cls.code_dir = SAMPLES_DIR
         cls.output_path = tempfile.mkstemp()[1]
+        cls.code_dir = SAMPLES_DIR
+        cls.code_filename = os.path.relpath(cls.code_path, SAMPLES_DIR)
+
+        if hasattr(cls, "expected_new_code"):
+            # Some tests are easier to understand with the expected new code provided
+            # instead of calculated
+            cls.original_code = "".join(_lines_from_codepath(cls.code_path))
+            cls.expected_new_code = dedent(cls.expected_new_code)
+        else:
+            cls.original_code, cls.expected_new_code = (
+                original_and_expected_from_code_path(
+                    cls.code_path, cls.replacement_lines
+                )
+            )
 
         # TODO: support sonar integration tests that add a dependency to
         # `requirements_file_name`. These tests would not be able to run

diff --git a/src/codemodder/codemods/utils_mixin.py b/src/codemodder/codemods/utils_mixin.py
@@ -535,7 +535,7 @@ def _resolve_name_transitive(self, node: cst.Name) -> Optional[cst.BaseExpressio
                         return self._resolve_name_transitive(value)
                     case _:
                         return value
-        return None
+        return node
 
     def _find_direct_name_assignment_targets(
         self, name: cst.Name
@@ -591,6 +591,82 @@ def find_transitive_assignment_targets(
             return named_targets, other_targets
         return ([], [])
 
+    def _resolve_starred_element(self, element: cst.StarredElement):
+        resolved = self.resolve_expression(element.value)
+        match resolved:
+            case cst.List():
+                return self.resolve_list_literal(resolved)
+        return [element]
+
+    def _resolve_list_element(self, element: cst.BaseElement):
+        match element:
+            case cst.Element():
+                return [self.resolve_expression(element.value)]
+            case cst.StarredElement():
+                return self._resolve_starred_element(element)
+        return []
+
+    def resolve_list_literal(
+        self, list_literal: cst.List
+    ) -> itertools.chain[cst.BaseExpression]:
+        """
+        Returns an iterable of all the elements of that list resolved. It will recurse into starred elements whenever possible.
+        """
+        return itertools.chain.from_iterable(
+            map(self._resolve_list_element, list_literal.elements)
+        )
+
+    def _resolve_starred_dict_element(self, element: cst.StarredDictElement):
+        resolved = self.resolve_expression(element.value)
+        match resolved:
+            case cst.Dict():
+                return self.resolve_dict(resolved)
+        return dict()
+
+    def _resolve_dict_element(self, element: cst.BaseDictElement):
+        match element:
+            case cst.StarredDictElement():
+                return self._resolve_starred_dict_element(element)
+            case _:
+                resolved_key = self.resolve_expression(element.key)
+                resolved_key_value = resolved_key
+                resolved_value = self.resolve_expression(element.value)
+                return {resolved_key_value: resolved_value}
+
+    def resolve_dict(self, dictionary: cst.Dict):
+        return {
+            k: v
+            for e in dictionary.elements
+            for k, v in self._resolve_dict_element(e).items()
+        }
+
+    def _transform_string_elements(self, expr):
+        match expr:
+            case cst.SimpleString():
+                return expr.raw_value
+        return expr
+
+    def resolve_keyword_args(self, call: cst.Call):
+        """
+        Returns a dict with all the keyword arguments resolved. It will recurse into starred elements whenever possible and string keys from double starred dict arguments will be converted into their raw_value.
+        """
+        keyword_to_expr_dict: dict = dict()
+        for arg in call.args:
+            if arg.star == "**":
+                resolved = self.resolve_expression(arg.value)
+                match resolved:
+                    case cst.Dict():
+                        resolved_starred_element = self.resolve_dict(resolved)
+                        keyword_to_expr_dict |= {
+                            self._transform_string_elements(k): v
+                            for k, v in resolved_starred_element.items()
+                        }
+            if arg.keyword:
+                keyword_to_expr_dict |= {
+                    arg.keyword.value: self.resolve_expression(arg.value)
+                }
+        return keyword_to_expr_dict
+
 
 def iterate_left_expressions(node: cst.BaseExpression):
     yield node

diff --git a/src/codemodder/scripts/generate_docs.py b/src/codemodder/scripts/generate_docs.py
@@ -271,6 +271,10 @@ class DocMetadata:
         importance="Low",
         guidance_explained="While this change will make the code consistent, it is likely that the `break` or `continue` statement is a symptom of an error in program logic.",
     ),
+    "disable-graphql-introspection": DocMetadata(
+        importance="High",
+        guidance_explained="This change may disable a feature that was left on purpose. Moreover the rule added may be framework specific.",
+    ),
 }
 DEFECTDOJO_CODEMODS = {
     "django-secure-set-cookie": DocMetadata(
@@ -305,6 +309,7 @@ class DocMetadata:
     "sql-parameterization-S3649",
     "django-model-without-dunder-str-S6554",
     "break-or-continue-out-of-loop-S1716",
+    "disable-graphql-introspection-S6786",
 ]
 SONAR_CODEMODS = {
     name: DocMetadata(

diff --git a/src/core_codemods/__init__.py b/src/core_codemods/__init__.py
@@ -8,6 +8,7 @@
     AvoidInsecureDeserialization,
 )
 from .defectdojo.semgrep.django_secure_set_cookie import DjangoSecureSetCookie
+from .disable_graphql_introspection import DisableGraphQLIntrospection
 from .django_debug_flag_on import DjangoDebugFlagOn
 from .django_json_response_type import DjangoJsonResponseType
 from .django_model_without_dunder_str import DjangoModelWithoutDunderStr
@@ -54,6 +55,7 @@
 from .secure_flask_session_config import SecureFlaskSessionConfig
 from .secure_random import SecureRandom
 from .sonar.sonar_break_or_continue_out_of_loop import SonarBreakOrContinueOutOfLoop
+from .sonar.sonar_disable_graphql_introspection import SonarDisableGraphQLIntrospection
 from .sonar.sonar_django_json_response_type import SonarDjangoJsonResponseType
 from .sonar.sonar_django_model_without_dunder_str import (
     SonarDjangoModelWithoutDunderStr,
@@ -153,6 +155,7 @@
         FixMissingSelfOrCls,
         FixMathIsClose,
         BreakOrContinueOutOfLoop,
+        DisableGraphQLIntrospection,
     ],
 )
 
@@ -178,6 +181,7 @@
         SonarSQLParameterization,
         SonarDjangoModelWithoutDunderStr,
         SonarBreakOrContinueOutOfLoop,
+        SonarDisableGraphQLIntrospection,
     ],
 )