diff --git a/integration_tests/test_sonar_fix_assert_tuple.py b/integration_tests/test_sonar_fix_assert_tuple.py index 2de689e3..e698d273 100644 --- a/integration_tests/test_sonar_fix_assert_tuple.py +++ b/integration_tests/test_sonar_fix_assert_tuple.py @@ -8,7 +8,6 @@ class TestFixAssertTuple(SonarIntegrationTest): code_path = "tests/samples/fix_assert_tuple.py" replacement_lines = [(1, "assert 1 == 1\n"), (2, "assert 2 == 2\n")] expected_diff = "--- \n+++ \n@@ -1 +1,2 @@\n-assert (1 == 1, 2 == 2)\n+assert 1 == 1\n+assert 2 == 2\n" - sonar_issues_json = "tests/samples/sonar_issues.json" expected_line_change = "1" change_description = FixAssertTupleTransform.change_description num_changes = 2 diff --git a/integration_tests/test_sonar_tempfile_mktemp.py b/integration_tests/test_sonar_tempfile_mktemp.py new file mode 100644 index 00000000..8282fc8e --- /dev/null +++ b/integration_tests/test_sonar_tempfile_mktemp.py @@ -0,0 +1,12 @@ +from codemodder.codemods.test import SonarIntegrationTest +from core_codemods.sonar.sonar_tempfile_mktemp import SonarTempfileMktemp +from core_codemods.tempfile_mktemp import TempfileMktempTransformer + + +class TestTempfileMktemp(SonarIntegrationTest): + codemod = SonarTempfileMktemp + code_path = "tests/samples/tempfile_mktemp.py" + replacement_lines = [(3, "tempfile.mkstemp()\n")] + expected_diff = "--- \n+++ \n@@ -1,3 +1,3 @@\n import tempfile\n \n-tempfile.mktemp()\n+tempfile.mkstemp()\n" + expected_line_change = "3" + change_description = TempfileMktempTransformer.change_description diff --git a/integration_tests/test_tempfile_mktemp.py b/integration_tests/test_tempfile_mktemp.py index 3ea2946d..cb9a17fe 100644 --- a/integration_tests/test_tempfile_mktemp.py +++ b/integration_tests/test_tempfile_mktemp.py @@ -1,5 +1,5 @@ from codemodder.codemods.test import BaseIntegrationTest -from core_codemods.tempfile_mktemp import TempfileMktemp +from core_codemods.tempfile_mktemp import TempfileMktemp, TempfileMktempTransformer class TestTempfileMktemp(BaseIntegrationTest): @@ -13,4 +13,4 @@ class TestTempfileMktemp(BaseIntegrationTest): replacement_lines = [(3, "tempfile.mkstemp()\n")] expected_diff = '--- \n+++ \n@@ -1,4 +1,4 @@\n import tempfile\n \n-tempfile.mktemp()\n+tempfile.mkstemp()\n var = "hello"\n' expected_line_change = "3" - change_description = TempfileMktemp.change_description + change_description = TempfileMktempTransformer.change_description diff --git a/src/codemodder/codemods/sonar.py b/src/codemodder/codemods/sonar.py index dbb7acae..a5d54153 100644 --- a/src/codemodder/codemods/sonar.py +++ b/src/codemodder/codemods/sonar.py @@ -14,6 +14,10 @@ class SonarCodemod(SASTCodemod): def origin(self): return "sonar" + @property + def rule_id(self): + return self._metadata.tool.rule_id + @classmethod def from_core_codemod( cls, diff --git a/src/codemodder/codemods/test/integration_utils.py b/src/codemodder/codemods/test/integration_utils.py index 15052953..4de44a9c 100644 --- a/src/codemodder/codemods/test/integration_utils.py +++ b/src/codemodder/codemods/test/integration_utils.py @@ -11,6 +11,7 @@ import jsonschema from codemodder import __version__, registry +from codemodder.sonar_results import SonarResultSet from .validations import execute_code @@ -115,19 +116,6 @@ def _assert_run_fields(self, run, output_path): assert run["directory"] == os.path.abspath(self.code_dir) assert run["sarifs"] == [] - def _assert_sonar_fields(self, result): - assert result["detectionTool"]["name"] == "Sonar" - assert ( - result["detectionTool"]["rule"]["id"] - == self.codemod_instance._metadata.tool.rule_id - ) - assert ( - result["detectionTool"]["rule"]["name"] - == self.codemod_instance._metadata.tool.rule_name - ) - # TODO: empty array until we add findings metadata - assert result["detectionTool"]["findings"] == [] - def _assert_results_fields(self, results, output_path): assert len(results) == 1 result = results[0] @@ -146,13 +134,7 @@ def _assert_results_fields(self, results, output_path): ]: assert reference["url"] == reference["description"] - if self.sonar_issues_json: - assert self.codemod_instance._metadata.tool is not None - assert ( - result["references"][-1]["description"] - == self.codemod_instance._metadata.tool.rule_name - ) - self._assert_sonar_fields(result) + self._assert_sonar_fields(result) assert len(result["changeset"]) == self.num_changed_files @@ -169,6 +151,9 @@ def _assert_results_fields(self, results, output_path): assert line_change["lineNumber"] == int(self.expected_line_change) assert line_change["description"] == self.change_description + def _assert_sonar_fields(self, result): + del result + def _assert_codetf_output(self, codetf_schema): with open(self.output_path, "r", encoding="utf-8") as f: codetf = json.load(f) @@ -277,6 +262,7 @@ def setup_class(cls): # in parallel at this time since they would all override the same # tests/samples/requirements.txt file, unless we change that to # a temporary file. + cls.check_sonar_issues() @classmethod def teardown_class(cls): @@ -286,6 +272,37 @@ def teardown_class(cls): with open(cls.code_path, mode="w", encoding="utf-8") as f: f.write(cls.original_code) + @classmethod + def check_sonar_issues(cls): + sonar_results = SonarResultSet.from_json(cls.sonar_issues_json) + + assert ( + cls.codemod.rule_id in sonar_results + ), f"Make sure to add a sonar issue for {cls.codemod.rule_id} in {cls.sonar_issues_json}" + results_for_codemod = sonar_results[cls.codemod.rule_id] + file_path = pathlib.Path(cls.code_filename) + assert ( + file_path in results_for_codemod + ), f"Make sure to add a sonar issue for file `{cls.code_filename}` under rule `{cls.codemod.rule_id}` in {cls.sonar_issues_json}" + + def _assert_sonar_fields(self, result): + assert self.codemod_instance._metadata.tool is not None + assert ( + result["references"][-1]["description"] + == self.codemod_instance._metadata.tool.rule_name + ) + assert result["detectionTool"]["name"] == "Sonar" + assert ( + result["detectionTool"]["rule"]["id"] + == self.codemod_instance._metadata.tool.rule_id + ) + assert ( + result["detectionTool"]["rule"]["name"] + == self.codemod_instance._metadata.tool.rule_name + ) + # TODO: empty array until we add findings metadata + assert result["detectionTool"]["findings"] == [] + def original_and_expected_from_code_path(code_path, replacements): """ diff --git a/src/codemodder/scripts/generate_docs.py b/src/codemodder/scripts/generate_docs.py index 975773a8..ca38ca4d 100644 --- a/src/codemodder/scripts/generate_docs.py +++ b/src/codemodder/scripts/generate_docs.py @@ -314,6 +314,11 @@ class DocMetadata: guidance_explained=CORE_METADATA["fix-missing-self-or-cls"].guidance_explained, need_sarif="Yes (Sonar)", ), + "secure-tempfile-S5445": DocMetadata( + importance=CORE_METADATA["secure-tempfile"].importance, + guidance_explained=CORE_METADATA["secure-tempfile"].guidance_explained, + need_sarif="Yes (Sonar)", + ), } diff --git a/src/core_codemods/__init__.py b/src/core_codemods/__init__.py index f5a731f5..99a4406d 100644 --- a/src/core_codemods/__init__.py +++ b/src/core_codemods/__init__.py @@ -57,6 +57,7 @@ from .sonar.sonar_remove_assertion_in_pytest_raises import ( SonarRemoveAssertionInPytestRaises, ) +from .sonar.sonar_tempfile_mktemp import SonarTempfileMktemp from .sql_parameterization import SQLQueryParameterization from .str_concat_in_seq_literal import StrConcatInSeqLiteral from .subprocess_shell_false import SubprocessShellFalse @@ -146,5 +147,6 @@ SonarDjangoJsonResponseType, SonarJwtDecodeVerify, SonarFixMissingSelfOrCls, + SonarTempfileMktemp, ], ) diff --git a/src/core_codemods/sonar/sonar_tempfile_mktemp.py b/src/core_codemods/sonar/sonar_tempfile_mktemp.py new file mode 100644 index 00000000..4348eed7 --- /dev/null +++ b/src/core_codemods/sonar/sonar_tempfile_mktemp.py @@ -0,0 +1,10 @@ +from codemodder.codemods.sonar import SonarCodemod +from core_codemods.tempfile_mktemp import TempfileMktemp + +SonarTempfileMktemp = SonarCodemod.from_core_codemod( + name="secure-tempfile-S5445", + other=TempfileMktemp, + rule_id="python:S5445", + rule_name="Insecure temporary file creation methods should not be used", + rule_url="https://rules.sonarsource.com/python/type/Vulnerability/RSPEC-5445/", +) diff --git a/src/core_codemods/tempfile_mktemp.py b/src/core_codemods/tempfile_mktemp.py index 6760b8c4..d441c28f 100644 --- a/src/core_codemods/tempfile_mktemp.py +++ b/src/core_codemods/tempfile_mktemp.py @@ -1,9 +1,26 @@ +from codemodder.codemods.libcst_transformer import ( + LibcstResultTransformer, + LibcstTransformerPipeline, +) +from codemodder.codemods.semgrep import SemgrepRuleDetector from codemodder.codemods.utils_mixin import NameResolutionMixin -from core_codemods.api import Metadata, Reference, ReviewGuidance, SimpleCodemod +from core_codemods.api import CoreCodemod, Metadata, Reference, ReviewGuidance -class TempfileMktemp(SimpleCodemod, NameResolutionMixin): - metadata = Metadata( +class TempfileMktempTransformer(LibcstResultTransformer, NameResolutionMixin): + change_description = "Replaces `tempfile.mktemp` with `tempfile.mkstemp`." + _module_name = "tempfile" + + def on_result_found(self, original_node, updated_node): + maybe_name = self.get_aliased_prefix_name(original_node, self._module_name) + if (maybe_name := maybe_name or self._module_name) == self._module_name: + self.add_needed_import(self._module_name) + self.remove_unused_import(original_node) + return self.update_call_target(updated_node, maybe_name, "mkstemp") + + +TempfileMktemp = CoreCodemod( + metadata=Metadata( name="secure-tempfile", summary="Upgrade and Secure Temp File Creation", review_guidance=ReviewGuidance.MERGE_WITHOUT_REVIEW, @@ -12,11 +29,9 @@ class TempfileMktemp(SimpleCodemod, NameResolutionMixin): url="https://docs.python.org/3/library/tempfile.html#tempfile.mktemp" ), ], - ) - change_description = "Replaces `tempfile.mktemp` with `tempfile.mkstemp`." - - _module_name = "tempfile" - detector_pattern = """ + ), + detector=SemgrepRuleDetector( + """ rules: - patterns: - pattern: tempfile.mktemp(...) @@ -24,10 +39,6 @@ class TempfileMktemp(SimpleCodemod, NameResolutionMixin): import tempfile ... """ - - def on_result_found(self, original_node, updated_node): - maybe_name = self.get_aliased_prefix_name(original_node, self._module_name) - if (maybe_name := maybe_name or self._module_name) == self._module_name: - self.add_needed_import(self._module_name) - self.remove_unused_import(original_node) - return self.update_call_target(updated_node, maybe_name, "mkstemp") + ), + transformer=LibcstTransformerPipeline(TempfileMktempTransformer), +) diff --git a/tests/codemods/test_sonar_tempfile_mktemp.py b/tests/codemods/test_sonar_tempfile_mktemp.py new file mode 100644 index 00000000..0075e8b3 --- /dev/null +++ b/tests/codemods/test_sonar_tempfile_mktemp.py @@ -0,0 +1,40 @@ +import json + +from codemodder.codemods.test import BaseSASTCodemodTest +from core_codemods.sonar.sonar_tempfile_mktemp import SonarTempfileMktemp + + +class TestSonarTempfileMktemp(BaseSASTCodemodTest): + codemod = SonarTempfileMktemp + tool = "sonar" + + def test_name(self): + assert self.codemod.name == "secure-tempfile-S5445" + + def test_simple(self, tmpdir): + input_code = """ + import tempfile + + tempfile.mktemp() + """ + expected = """ + import tempfile + + tempfile.mkstemp() + """ + issues = { + "issues": [ + { + "rule": "python:S5445", + "status": "OPEN", + "component": "code.py", + "textRange": { + "startLine": 4, + "endLine": 4, + "startOffset": 0, + "endOffset": 17, + }, + } + ] + } + self.run_and_assert(tmpdir, input_code, expected, results=json.dumps(issues)) diff --git a/tests/samples/tempfile_mktemp.py b/tests/samples/tempfile_mktemp.py new file mode 100644 index 00000000..f6e80500 --- /dev/null +++ b/tests/samples/tempfile_mktemp.py @@ -0,0 +1,3 @@ +import tempfile + +tempfile.mktemp()