Support sonar hotspots and secure-random sonar hotspot rule

integration_tests/test_secure_random.py

@@ -1,5 +1,5 @@
 from codemodder.codemods.test import BaseIntegrationTest
-from core_codemods.secure_random import SecureRandom
+from core_codemods.secure_random import SecureRandom, SecureRandomTransformer
 
 
 class TestSecureRandom(BaseIntegrationTest):
@@ -26,4 +26,4 @@ class TestSecureRandom(BaseIntegrationTest):
         """ var = "hello"\n""")
     # fmt: on
     expected_line_change = "2"
-    change_description = SecureRandom.change_description
+    change_description = SecureRandomTransformer.change_description

integration_tests/test_sonar_secure_random.py

@@ -0,0 +1,29 @@
+from codemodder.codemods.test import SonarIntegrationTest
+from core_codemods.secure_random import SecureRandomTransformer
+from core_codemods.sonar.sonar_secure_random import SonarSecureRandom
+
+
+class TestSonarDjangoJsonResponseType(SonarIntegrationTest):
+    codemod = SonarSecureRandom
+    code_path = "tests/samples/secure_random.py"
+    replacement_lines = [
+        (1, """import secrets\n"""),
+        (3, """secrets.SystemRandom().random()\n"""),
+        (4, """secrets.SystemRandom().getrandbits(1)\n"""),
+    ]
+    # fmt: off
+    expected_diff = (
+        """--- \n"""
+        """+++ \n"""
+        """@@ -1,4 +1,4 @@\n"""
+        """-import random\n"""
+        """+import secrets\n"""
+        """ \n"""
+        """-random.random()\n"""
+        """-random.getrandbits(1)\n"""
+        """+secrets.SystemRandom().random()\n"""
+        """+secrets.SystemRandom().getrandbits(1)\n""")
+    # fmt: on
+    expected_line_change = "3"
+    change_description = SecureRandomTransformer.change_description
+    num_changes = 2

src/codemodder/codemodder.py

@@ -4,7 +4,7 @@
 import os
 import sys
 from pathlib import Path
-from typing import Sequence
+from typing import DefaultDict, Sequence
 
 from codemodder import __version__, registry
 from codemodder.cli import parse_args
@@ -158,10 +158,20 @@ def run(original_args) -> int:
 
     # TODO: sonar files should be _parsed_ here as well
     # TODO: this should be dict[str, list[Path]]
-    tool_result_files_map: dict[str, list[str]] = detect_sarif_tools(
+
+    tool_result_files_map: DefaultDict[str, list[str]] = detect_sarif_tools(
         [Path(name) for name in argv.sarif or []]
     )
-    tool_result_files_map["sonar"] = argv.sonar_issues_json
+    (
+        tool_result_files_map["sonar"].extend(argv.sonar_issues_json)
+        if argv.sonar_issues_json
+        else None
+    )
+    (
+        tool_result_files_map["sonar"].extend(argv.sonar_hotspots_json)
+        if argv.sonar_hotspots_json
+        else None
+    )
 
     repo_manager = PythonRepoManager(Path(argv.directory))
     context = CodemodExecutionContext(

src/codemodder/codemods/sonar.py

@@ -54,20 +54,21 @@ def from_core_codemod(
 class SonarDetector(BaseDetector):
     _lazy_cache = None
 
-    def _process_sonar_findings(self, sonar_json_files: list[str]) -> SonarResultSet:
-        combined_result_set = SonarResultSet()
-        for file in sonar_json_files or []:
-            combined_result_set |= SonarResultSet.from_json(file)
-        return combined_result_set
-
     def apply(
         self,
         codemod_id: str,
         context: CodemodExecutionContext,
         files_to_analyze: list[Path],
     ) -> ResultSet:
         if not self._lazy_cache:
-            self._lazy_cache = self._process_sonar_findings(
+            self._lazy_cache = process_sonar_findings(
                 context.tool_result_files_map.get("sonar", [])
             )
         return self._lazy_cache
+
+
+def process_sonar_findings(sonar_json_files: list[str]) -> SonarResultSet:
+    combined_result_set = SonarResultSet()
+    for file in sonar_json_files or []:
+        combined_result_set |= SonarResultSet.from_json(file)
+    return combined_result_set

src/codemodder/codemods/test/integration_utils.py

@@ -11,7 +11,6 @@
 import jsonschema
 
 from codemodder import __version__, registry
-from codemodder.sonar_results import SonarResultSet
 
 from .validations import execute_code
 
@@ -43,6 +42,7 @@ class BaseIntegrationTest(DependencyTestMixin):
     num_changed_files = 1
     allowed_exceptions = ()
     sonar_issues_json: str | None = None
+    sonar_hotspots_json: str | None = None
 
     @classmethod
     def setup_class(cls):
@@ -112,6 +112,10 @@ def _assert_run_fields(self, run, output_path):
             f" --sonar-issues-json={self.sonar_issues_json}"
             if self.sonar_issues_json
             else ""
+        ) + (
+            f" --sonar-hotspots-json={self.sonar_hotspots_json}"
+            if self.sonar_hotspots_json
+            else ""
         )
         assert run["directory"] == os.path.abspath(self.code_dir)
         assert run["sarifs"] == []
@@ -126,11 +130,12 @@ def _assert_results_fields(self, results, output_path):
         ]
 
         assert ("detectionTool" in result) == bool(self.sonar_issues_json)
+        assert ("detectionTool" in result) == bool(self.sonar_hotspots_json)
 
         # TODO: if/when we add description for each url
         for reference in result["references"][
             # Last reference for Sonar has a different description
-            : (-1 if self.sonar_issues_json else None)
+            : (-1 if self.sonar_issues_json or self.sonar_hotspots_json else None)
         ]:
             assert reference["url"] == reference["description"]
 
@@ -199,6 +204,8 @@ def test_file_rewritten(self, codetf_schema):
 
         if self.sonar_issues_json:
             command.append(f"--sonar-issues-json={self.sonar_issues_json}")
+        if self.sonar_hotspots_json:
+            command.append(f"--sonar-hotspots-json={self.sonar_hotspots_json}")
 
         self.write_original_code()
         self.write_original_dependencies()
@@ -245,6 +252,7 @@ class SonarIntegrationTest(BaseIntegrationTest):
 
     code_path = NotImplementedError
     sonar_issues_json = "tests/samples/sonar_issues.json"
+    sonar_hotspots_json = "tests/samples/sonar_hotspots.json"
 
     @classmethod
     def setup_class(cls):
@@ -274,16 +282,20 @@ def teardown_class(cls):
 
     @classmethod
     def check_sonar_issues(cls):
-        sonar_results = SonarResultSet.from_json(cls.sonar_issues_json)
+        from codemodder.codemods.sonar import process_sonar_findings
+
+        sonar_results = process_sonar_findings(
+            [cls.sonar_issues_json, cls.sonar_hotspots_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}"
+        ), f"Make sure to add a sonar issue/hotspot for {cls.codemod.rule_id} in {cls.sonar_issues_json} or {cls.sonar_hotspots_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}"
+        ), f"Make sure to add a sonar issue/hotspot for file `{cls.code_filename}` under rule `{cls.codemod.rule_id}`in {cls.sonar_issues_json} or {cls.sonar_hotspots_json}"
 
     def _assert_sonar_fields(self, result):
         assert self.codemod_instance._metadata.tool is not None

src/codemodder/sarifs.py

@@ -1,8 +1,9 @@
 import json
 from abc import ABCMeta, abstractmethod
+from collections import defaultdict
 from importlib.metadata import entry_points
 from pathlib import Path
-from typing import Optional
+from typing import DefaultDict, Optional
 
 from typing_extensions import Self
 
@@ -18,8 +19,8 @@ def detect(cls, run_data: dict) -> bool:
         pass
 
 
-def detect_sarif_tools(filenames: list[Path]) -> dict[str, list[str]]:
-    results: dict[str, list[str]] = {}
+def detect_sarif_tools(filenames: list[Path]) -> DefaultDict[str, list[str]]:
+    results: DefaultDict[str, list[str]] = defaultdict(list)
 
     logger.debug("loading registered SARIF tool detectors")
     detectors = {
@@ -33,7 +34,7 @@ def detect_sarif_tools(filenames: list[Path]) -> dict[str, list[str]]:
                 try:
                     if det.detect(run):
                         logger.debug("detected %s sarif: %s", name, fname)
-                        results.setdefault(name, []).append(str(fname))
+                        results[name].append(str(fname))
                 except (KeyError, AttributeError, ValueError):
                     continue
 

src/codemodder/scripts/generate_docs.py

@@ -319,6 +319,11 @@ class DocMetadata:
         guidance_explained=CORE_METADATA["secure-tempfile"].guidance_explained,
         need_sarif="Yes (Sonar)",
     ),
+    "secure-random-S2245": DocMetadata(
+        importance=CORE_METADATA["secure-random"].importance,
+        guidance_explained=CORE_METADATA["secure-random"].guidance_explained,
+        need_sarif="Yes (Sonar)",
+    ),
 }
 
 

src/codemodder/sonar_results.py

@@ -12,23 +12,23 @@
 
 class SonarLocation(Location):
     @classmethod
-    def from_issue(cls, issue) -> Self:
-        location = issue.get("textRange")
+    def from_result(cls, result) -> Self:
+        location = result.get("textRange")
         start = LineInfo(location.get("startLine"), location.get("startOffset"), "")
         end = LineInfo(location.get("endLine"), location.get("endOffset"), "")
-        file = Path(issue.get("component").split(":")[-1])
+        file = Path(result.get("component").split(":")[-1])
         return cls(file=file, start=start, end=end)
 
 
 class SonarResult(Result):
 
     @classmethod
-    def from_issue(cls, issue) -> Self:
-        rule_id = issue.get("rule", None)
-        if not rule_id:
+    def from_result(cls, result) -> Self:
+        # Sonar issues have `rule` as key while hotspots call it `ruleKey`
+        if not (rule_id := result.get("rule", None) or result.get("ruleKey", None)):
             raise ValueError("Could not extract rule id from sarif result.")
 
-        locations: list[Location] = [SonarLocation.from_issue(issue)]
+        locations: list[Location] = [SonarLocation.from_result(result)]
         return cls(rule_id=rule_id, locations=locations)
 
     def match_location(self, pos, node):
@@ -51,9 +51,9 @@ def from_json(cls, json_file: str | Path) -> Self:
                 data = json.load(file)
 
             result_set = cls()
-            for issue in data.get("issues"):
-                if issue["status"].lower() == "open":
-                    result_set.add_result(SonarResult.from_issue(issue))
+            for result in data.get("issues") or [] + data.get("hotspots") or []:
+                if result["status"].lower() in ("open", "to_review"):
+                    result_set.add_result(SonarResult.from_result(result))
 
             return result_set
         except Exception:

src/core_codemods/__init__.py

@@ -57,6 +57,7 @@
 from .sonar.sonar_remove_assertion_in_pytest_raises import (
     SonarRemoveAssertionInPytestRaises,
 )
+from .sonar.sonar_secure_random import SonarSecureRandom
 from .sonar.sonar_tempfile_mktemp import SonarTempfileMktemp
 from .sql_parameterization import SQLQueryParameterization
 from .str_concat_in_seq_literal import StrConcatInSeqLiteral
@@ -148,5 +149,6 @@
         SonarJwtDecodeVerify,
         SonarFixMissingSelfOrCls,
         SonarTempfileMktemp,
+        SonarSecureRandom,
     ],
 )

src/core_codemods/secure_random.py

@@ -1,8 +1,28 @@
-from core_codemods.api import Metadata, Reference, ReviewGuidance, SimpleCodemod
+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 CoreCodemod, Metadata, Reference, ReviewGuidance
 
 
-class SecureRandom(SimpleCodemod):
-    metadata = Metadata(
+class SecureRandomTransformer(LibcstResultTransformer, NameResolutionMixin):
+    change_description = (
+        "Replace random.{func} with more secure secrets library functions."
+    )
+
+    def on_result_found(self, original_node, updated_node):
+        self.remove_unused_import(original_node)
+        self.add_needed_import("secrets")
+
+        if self.find_base_name(original_node.func) == "random.choice":
+            return self.update_call_target(updated_node, "secrets")
+        return self.update_call_target(updated_node, "secrets.SystemRandom()")
+
+
+SecureRandom = CoreCodemod(
+    metadata=Metadata(
         name="secure-random",
         review_guidance=ReviewGuidance.MERGE_AFTER_CURSORY_REVIEW,
         summary="Secure Source of Randomness",
@@ -14,22 +34,16 @@ class SecureRandom(SimpleCodemod):
                 url="https://docs.python.org/3/library/random.html",
             ),
         ],
-    )
-
-    detector_pattern = """
-        - patterns:
-          - pattern: random.$F(...)
-          - pattern-not: random.SystemRandom()
-          - pattern-inside: |
-              import random
-              ...
-    """
-
-    change_description = (
-        "Replace random.{func} with more secure secrets library functions."
-    )
-
-    def on_result_found(self, original_node, updated_node):
-        self.remove_unused_import(original_node)
-        self.add_needed_import("secrets")
-        return self.update_call_target(updated_node, "secrets.SystemRandom()")
+    ),
+    detector=SemgrepRuleDetector(
+        """
+            - patterns:
+              - pattern: random.$F(...)
+              - pattern-not: random.SystemRandom()
+              - pattern-inside: |
+                  import random
+                  ...
+        """
+    ),
+    transformer=LibcstTransformerPipeline(SecureRandomTransformer),
+)

src/core_codemods/sonar/sonar_secure_random.py

@@ -0,0 +1,10 @@
+from codemodder.codemods.sonar import SonarCodemod
+from core_codemods.secure_random import SecureRandom
+
+SonarSecureRandom = SonarCodemod.from_core_codemod(
+    name="secure-random-S2245",
+    other=SecureRandom,
+    rule_id="python:S2245",
+    rule_name="Using pseudorandom number generators (PRNGs) is security-sensitive",
+    rule_url="https://rules.sonarsource.com/python/type/Security%20Hotspot/RSPEC-2245/",
+)