Skip to content

Commit

Permalink
Sonar version of flask-json-response-type and UtilsMixin refactor
Browse files Browse the repository at this point in the history
  • Loading branch information
andrecsilva committed Feb 5, 2024
1 parent fe47276 commit a5406e8
Show file tree
Hide file tree
Showing 14 changed files with 227 additions and 59 deletions.
7 changes: 5 additions & 2 deletions integration_tests/test_flask_json_response_type.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
from core_codemods.flask_json_response_type import FlaskJsonResponseType
from core_codemods.flask_json_response_type import (
FlaskJsonResponseType,
FlaskJsonResponseTypeTransformer,
)
from integration_tests.base_test import (
BaseIntegrationTest,
original_and_expected_from_code_path,
Expand Down Expand Up @@ -32,5 +35,5 @@ class TestFlaskJsonResponseType(BaseIntegrationTest):
# fmt: on

expected_line_change = "9"
change_description = FlaskJsonResponseType.change_description
change_description = FlaskJsonResponseTypeTransformer.change_description
num_changed_files = 1
40 changes: 40 additions & 0 deletions integration_tests/test_sonar_flask_json_response_type.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
from core_codemods.flask_json_response_type import FlaskJsonResponseTypeTransformer
from core_codemods.sonar.sonar_flask_json_response_type import (
SonarFlaskJsonResponseType,
)
from integration_tests.base_test import (
BaseIntegrationTest,
original_and_expected_from_code_path,
)


class TestSonarFlaskJsonResponseType(BaseIntegrationTest):
codemod = SonarFlaskJsonResponseType
code_path = "tests/samples/flask_json_response_type.py"
original_code, expected_new_code = original_and_expected_from_code_path(
code_path,
[
(
8,
""" return make_response(json_response, {'Content-Type': 'application/json'})\n""",
),
],
)
sonar_issues_json = "tests/samples/sonar_issues.json"

# fmt: off
expected_diff =(
"""--- \n"""
"""+++ \n"""
"""@@ -6,4 +6,4 @@\n"""
""" @app.route("/test")\n"""
""" def foo(request):\n"""
""" json_response = json.dumps({ "user_input": request.GET.get("input") })\n"""
"""- return make_response(json_response)\n"""
"""+ return make_response(json_response, {'Content-Type': 'application/json'})\n"""
)
# fmt: on

expected_line_change = "9"
change_description = FlaskJsonResponseTypeTransformer.change_description
num_changed_files = 1
42 changes: 30 additions & 12 deletions src/codemodder/codemods/base_visitor.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,24 @@
from typing import Any, Tuple
from libcst import MetadataDependent
from libcst.codemod import ContextAwareVisitor, VisitorBasedCodemodCommand
from libcst.metadata import PositionProvider

from codemodder.result import Result


# TODO: this should just be part of BaseTransformer and BaseVisitor?
class UtilsMixin:
results: list[Result] | None
class UtilsMixin(MetadataDependent):
METADATA_DEPENDENCIES: Tuple[Any, ...] = (PositionProvider,)

def __init__(self, results: list[Result] | None):
def __init__(
self,
results: list[Result] | None,
line_exclude: list[int],
line_include: list[int],
): # pylint: disable=super-init-not-called
self.results = results
self.line_exclude = line_exclude
self.line_include = line_include

def filter_by_result(self, node):
pos_to_match = self.node_position(node)
Expand All @@ -37,26 +45,36 @@ def node_is_selected(self, node) -> bool:

def node_position(self, node):
# See https://github.com/Instagram/LibCST/blob/main/libcst/_metadata_dependent.py#L112
return self.get_metadata(self.METADATA_DEPENDENCIES[0], node)
return self.get_metadata(PositionProvider, node)

def lineno_for_node(self, node):
return self.node_position(node).start.line


class BaseTransformer(VisitorBasedCodemodCommand, UtilsMixin):
METADATA_DEPENDENCIES: Tuple[Any, ...] = (PositionProvider,)

def __init__(self, context, results: list[Result] | None):
super().__init__(context)
UtilsMixin.__init__(self, results)
def __init__(
self,
context,
results: list[Result] | None,
line_include: list[int],
line_exclude: list[int],
):
VisitorBasedCodemodCommand.__init__(self, context)
UtilsMixin.__init__(self, results, line_exclude, line_include)


class BaseVisitor(ContextAwareVisitor, UtilsMixin):
METADATA_DEPENDENCIES: Tuple[Any, ...] = (PositionProvider,)

def __init__(self, context, results: list[Result]):
super().__init__(context)
UtilsMixin.__init__(self, results)
def __init__(
self,
context,
results: list[Result] | None,
line_include: list[int],
line_exclude: list[int],
):
ContextAwareVisitor.__init__(self, context)
UtilsMixin.__init__(self, results, line_exclude, line_include)


def match_line(pos, line):
Expand Down
15 changes: 6 additions & 9 deletions src/codemodder/codemods/libcst_transformer.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,12 @@ def __init__(
):
del _transformer

super().__init__(context, results)
super().__init__(
context,
results,
line_include=file_context.line_include,
line_exclude=file_context.line_exclude,
)
self.file_context = file_context

@classmethod
Expand Down Expand Up @@ -119,14 +124,6 @@ def add_change_from_position(
def lineno_for_node(self, node):
return self.node_position(node).start.line

@property
def line_exclude(self):
return self.file_context.line_exclude

@property
def line_include(self):
return self.file_context.line_include

def add_dependency(self, dependency: Dependency):
self.file_context.add_dependency(dependency)

Expand Down
5 changes: 5 additions & 0 deletions src/codemodder/scripts/generate_docs.py
Original file line number Diff line number Diff line change
Expand Up @@ -259,6 +259,11 @@ class DocMetadata:
].guidance_explained,
need_sarif="Yes (Sonar)",
),
"flask-json-response-type-S5131": DocMetadata(
importance=CORE_METADATA["flask-json-response-type"].importance,
guidance_explained=CORE_METADATA["flask-json-response-type"].guidance_explained,
need_sarif="Yes (Sonar)",
),
}


Expand Down
2 changes: 2 additions & 0 deletions src/core_codemods/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
from .sonar.sonar_remove_assertion_in_pytest_raises import (
SonarRemoveAssertionInPytestRaises,
)
from .sonar.sonar_flask_json_response_type import SonarFlaskJsonResponseType

registry = CodemodCollection(
origin="pixee",
Expand Down Expand Up @@ -127,5 +128,6 @@
SonarExceptionWithoutRaise,
SonarFixAssertTuple,
SonarRemoveAssertionInPytestRaises,
SonarFlaskJsonResponseType,
],
)
71 changes: 51 additions & 20 deletions src/core_codemods/flask_json_response_type.py
Original file line number Diff line number Diff line change
@@ -1,37 +1,35 @@
from typing import Optional, Tuple
import libcst as cst
from libcst.codemod import CodemodContext, ContextAwareVisitor
from codemodder.codemods.base_visitor import UtilsMixin
from codemodder.codemods.libcst_transformer import (
LibcstResultTransformer,
LibcstTransformerPipeline,
)

from codemodder.codemods.utils_mixin import NameAndAncestorResolutionMixin
from codemodder.file_context import FileContext
from codemodder.result import Result
from core_codemods.api import (
Metadata,
Reference,
ReviewGuidance,
SimpleCodemod,
)
from core_codemods.api.core_codemod import CoreCodemod


class FlaskJsonResponseType(SimpleCodemod, NameAndAncestorResolutionMixin):
metadata = Metadata(
name="flask-json-response-type",
summary="Set content type to `application/json` for `flask.make_response` with JSON data",
review_guidance=ReviewGuidance.MERGE_WITHOUT_REVIEW,
references=[
Reference(
url="https://flask.palletsprojects.com/en/2.3.x/patterns/javascript/#return-json-from-views"
),
Reference(
url="https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html#output-encoding-for-javascript-contexts"
),
],
)
class FlaskJsonResponseTypeTransformer(
LibcstResultTransformer, NameAndAncestorResolutionMixin
):
change_description = "Sets `mimetype` to `application/json`."

content_type_key = "Content-Type"
json_content_type = "application/json"

def transform_module_impl(self, tree: cst.Module) -> cst.Module:
visitor = FlaskJsonResponseTypeVisitor(self.context)
visitor = FlaskJsonResponseTypeVisitor(
self.context, file_context=self.file_context, results=self.results
)
tree.visit(visitor)
if visitor.node_and_replacement:
node, replacement = visitor.node_and_replacement
Expand All @@ -40,16 +38,30 @@ def transform_module_impl(self, tree: cst.Module) -> cst.Module:
return tree


class FlaskJsonResponseTypeVisitor(ContextAwareVisitor, NameAndAncestorResolutionMixin):
class FlaskJsonResponseTypeVisitor(
ContextAwareVisitor, NameAndAncestorResolutionMixin, UtilsMixin
):
content_type_key = "Content-Type"
json_content_type = "application/json"

def __init__(self, context: CodemodContext) -> None:
def __init__(
self,
context: CodemodContext,
file_context: FileContext,
results: list[Result] | None,
) -> None:
self.node_and_replacement: Optional[Tuple[cst.CSTNode, cst.CSTNode]] = None
super().__init__(context)
self.file_context = file_context
ContextAwareVisitor.__init__(self, context)
UtilsMixin.__init__(
self,
results=results,
line_include=file_context.line_include,
line_exclude=file_context.line_exclude,
)

def leave_Return(self, original_node: cst.Return):
if original_node.value:
if original_node.value and self.node_is_selected(original_node.value):
# is inside a function def with a route decorator
maybe_function_def = self.find_immediate_function_def(original_node)
maybe_has_decorator = (
Expand Down Expand Up @@ -224,3 +236,22 @@ def _fix_make_response(self, call: cst.Call) -> cst.Call:

def _fix_json_dumps(self, node: cst.BaseExpression) -> cst.Tuple:
return cst.Tuple([cst.Element(node), cst.Element(self._build_dict())])


FlaskJsonResponseType = CoreCodemod(
metadata=Metadata(
name="flask-json-response-type",
summary="Set content type to `application/json` for `flask.make_response` with JSON data",
review_guidance=ReviewGuidance.MERGE_WITHOUT_REVIEW,
references=[
Reference(
url="https://flask.palletsprojects.com/en/2.3.x/patterns/javascript/#return-json-from-views"
),
Reference(
url="https://cheatsheetseries.owasp.org/cheatsheets/Cross_Site_Scripting_Prevention_Cheat_Sheet.html#output-encoding-for-javascript-contexts"
),
],
),
transformer=LibcstTransformerPipeline(FlaskJsonResponseTypeTransformer),
detector=None,
)
4 changes: 3 additions & 1 deletion src/core_codemods/secure_flask_session_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,9 @@ class FixFlaskConfig(BaseTransformer, NameResolutionMixin):
}

def __init__(self, codemod_context: CodemodContext, file_context: FileContext):
super().__init__(codemod_context, [])
super().__init__(
codemod_context, [], file_context.line_include, file_context.line_exclude
)
self.flask_app_name = ""
# Later: if we want to store configs to write later
# self.configs_to_write = self.SECURE_SESSION_CONFIGS.copy()
Expand Down
12 changes: 12 additions & 0 deletions src/core_codemods/sonar/sonar_flask_json_response_type.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
from codemodder.codemods.base_codemod import Reference
from codemodder.codemods.sonar import SonarCodemod
from core_codemods.flask_json_response_type import FlaskJsonResponseType

SonarFlaskJsonResponseType = SonarCodemod.from_core_codemod(
name="flask-json-response-type-S5131",
other=FlaskJsonResponseType,
rules=["pythonsecurity:S5131"],
new_references=[
Reference(url="https://rules.sonarsource.com/python/type/Bug/RSPEC-5131/"),
],
)
7 changes: 6 additions & 1 deletion src/core_codemods/sql_parameterization.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,12 @@ def __init__(
cst.CSTNode | cst.RemovalSentinel | cst.FlattenSentinel | dict[str, Any],
] = {}
SimpleCodemod.__init__(self, *codemod_args, **codemod_kwargs)
UtilsMixin.__init__(self, [])
UtilsMixin.__init__(
self,
[],
line_exclude=self.file_context.line_exclude,
line_include=self.file_context.line_include,
)

def _build_param_element(self, prepend, middle, append):
new_middle = (
Expand Down
18 changes: 11 additions & 7 deletions src/core_codemods/url_sandbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import libcst as cst
from libcst import CSTNode, matchers
from libcst.codemod import CodemodContext
from libcst.codemod import CodemodContext, ContextAwareVisitor
from libcst.metadata import PositionProvider, ScopeProvider

from libcst.codemod.visitors import AddImportsVisitor, ImportItem
Expand All @@ -13,7 +13,7 @@
Reference,
ReviewGuidance,
)
from codemodder.codemods.base_visitor import BaseVisitor
from codemodder.codemods.base_visitor import UtilsMixin
from codemodder.codemods.transformations.remove_unused_imports import (
RemoveUnusedImportsCodemod,
)
Expand Down Expand Up @@ -98,19 +98,23 @@ def transform_module_impl(self, tree: cst.Module) -> cst.Module:
return tree


class FindRequestCallsAndImports(BaseVisitor):
METADATA_DEPENDENCIES = (*BaseVisitor.METADATA_DEPENDENCIES, ScopeProvider)
class FindRequestCallsAndImports(ContextAwareVisitor, UtilsMixin):
METADATA_DEPENDENCIES = (ScopeProvider,)

def __init__(
self, codemod_context: CodemodContext, file_context: FileContext, results
):
super().__init__(codemod_context, results)
self.line_exclude = file_context.line_exclude
self.line_include = file_context.line_include
self.nodes_to_change: dict[
cst.CSTNode, Union[cst.CSTNode, cst.FlattenSentinel, cst.RemovalSentinel]
] = {}
self.changes_in_file: List[Change] = []
ContextAwareVisitor.__init__(self, codemod_context)
UtilsMixin.__init__(
self,
results=results,
line_include=file_context.line_include,
line_exclude=file_context.line_exclude,
)

def leave_Call(self, original_node: cst.Call):
if not (self.node_is_selected(original_node)):
Expand Down
6 changes: 3 additions & 3 deletions tests/codemods/test_base_visitor.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ class DeleteStatementLinesCodemod(BaseTransformer):
METADATA_DEPENDENCIES = (PositionProvider,)

def __init__(self, context, results, line_exclude=None, line_include=None):
BaseTransformer.__init__(self, context, results)
self.line_exclude = line_exclude or []
self.line_include = line_include or []
BaseTransformer.__init__(
self, context, results, line_include or [], line_exclude or []
)

def filter_by_result(self, node):
return True
Expand Down
Loading

0 comments on commit a5406e8

Please sign in to comment.