Sonar tmpfile codemod (#393)

* new sonar tmpfile codemod

* update integration tests

* add assertions for sonar integration test

* add docs

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

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

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

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,

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):
     """

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)",
+    ),
 }
 
 

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,
     ],
 )

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/",
+)

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,22 +29,16 @@ 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(...)
             - pattern-inside: |
                 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),
+)

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))

tests/samples/tempfile_mktemp.py

@@ -0,0 +1,3 @@
+import tempfile
+
+tempfile.mktemp()