Skip to content

Commit 0d9fd81

Browse files
clavedelunaclavedeluna
authored
Support sonar hotspots and secure-random sonar hotspot rule (#433)
* add more secure random unit tests * random.choice can just be sampling.choice * start sonar secure random rule * support hotspots for sonar * make sonar defaultdict * add sonar hotspots * be able to test hotspots * add docs
1 parent 94dc982 commit 0d9fd81

15 files changed

Lines changed: 316 additions & 54 deletions

‎integration_tests/test_secure_random.py‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
from codemodder.codemods.test import BaseIntegrationTest
2-
from core_codemods.secure_random import SecureRandom
2+
from core_codemods.secure_random import SecureRandom, SecureRandomTransformer
33

44

55
class TestSecureRandom(BaseIntegrationTest):
@@ -26,4 +26,4 @@ class TestSecureRandom(BaseIntegrationTest):
2626
""" var = "hello"\n""")
2727
# fmt: on
2828
expected_line_change = "2"
29-
change_description = SecureRandom.change_description
29+
change_description = SecureRandomTransformer.change_description

‎integration_tests/test_sonar_secure_random.py‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
from codemodder.codemods.test import SonarIntegrationTest
2+
from core_codemods.secure_random import SecureRandomTransformer
3+
from core_codemods.sonar.sonar_secure_random import SonarSecureRandom
4+
5+
6+
class TestSonarDjangoJsonResponseType(SonarIntegrationTest):
7+
codemod = SonarSecureRandom
8+
code_path = "tests/samples/secure_random.py"
9+
replacement_lines = [
10+
(1, """import secrets\n"""),
11+
(3, """secrets.SystemRandom().random()\n"""),
12+
(4, """secrets.SystemRandom().getrandbits(1)\n"""),
13+
]
14+
# fmt: off
15+
expected_diff = (
16+
"""--- \n"""
17+
"""+++ \n"""
18+
"""@@ -1,4 +1,4 @@\n"""
19+
"""-import random\n"""
20+
"""+import secrets\n"""
21+
""" \n"""
22+
"""-random.random()\n"""
23+
"""-random.getrandbits(1)\n"""
24+
"""+secrets.SystemRandom().random()\n"""
25+
"""+secrets.SystemRandom().getrandbits(1)\n""")
26+
# fmt: on
27+
expected_line_change = "3"
28+
change_description = SecureRandomTransformer.change_description
29+
num_changes = 2

‎src/codemodder/codemodder.py‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
import os
55
import sys
66
from pathlib import Path
7-
from typing import Sequence
7+
from typing import DefaultDict, Sequence
88

99
from codemodder import __version__, registry
1010
from codemodder.cli import parse_args
@@ -158,10 +158,20 @@ def run(original_args) -> int:
158158

159159
# TODO: sonar files should be _parsed_ here as well
160160
# TODO: this should be dict[str, list[Path]]
161-
tool_result_files_map: dict[str, list[str]] = detect_sarif_tools(
161+
162+
tool_result_files_map: DefaultDict[str, list[str]] = detect_sarif_tools(
162163
[Path(name) for name in argv.sarif or []]
163164
)
164-
tool_result_files_map["sonar"] = argv.sonar_issues_json
165+
(
166+
tool_result_files_map["sonar"].extend(argv.sonar_issues_json)
167+
if argv.sonar_issues_json
168+
else None
169+
)
170+
(
171+
tool_result_files_map["sonar"].extend(argv.sonar_hotspots_json)
172+
if argv.sonar_hotspots_json
173+
else None
174+
)
165175

166176
repo_manager = PythonRepoManager(Path(argv.directory))
167177
context = CodemodExecutionContext(

‎src/codemodder/codemods/sonar.py‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -54,20 +54,21 @@ def from_core_codemod(
5454
class SonarDetector(BaseDetector):
5555
_lazy_cache = None
5656

57-
def _process_sonar_findings(self, sonar_json_files: list[str]) -> SonarResultSet:
58-
combined_result_set = SonarResultSet()
59-
for file in sonar_json_files or []:
60-
combined_result_set |= SonarResultSet.from_json(file)
61-
return combined_result_set
62-
6357
def apply(
6458
self,
6559
codemod_id: str,
6660
context: CodemodExecutionContext,
6761
files_to_analyze: list[Path],
6862
) -> ResultSet:
6963
if not self._lazy_cache:
70-
self._lazy_cache = self._process_sonar_findings(
64+
self._lazy_cache = process_sonar_findings(
7165
context.tool_result_files_map.get("sonar", [])
7266
)
7367
return self._lazy_cache
68+
69+
70+
def process_sonar_findings(sonar_json_files: list[str]) -> SonarResultSet:
71+
combined_result_set = SonarResultSet()
72+
for file in sonar_json_files or []:
73+
combined_result_set |= SonarResultSet.from_json(file)
74+
return combined_result_set

‎src/codemodder/codemods/test/integration_utils.py‎

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
import jsonschema
1212

1313
from codemodder import __version__, registry
14-
from codemodder.sonar_results import SonarResultSet
1514

1615
from .validations import execute_code
1716

@@ -43,6 +42,7 @@ class BaseIntegrationTest(DependencyTestMixin):
4342
num_changed_files = 1
4443
allowed_exceptions = ()
4544
sonar_issues_json: str | None = None
45+
sonar_hotspots_json: str | None = None
4646

4747
@classmethod
4848
def setup_class(cls):
@@ -112,6 +112,10 @@ def _assert_run_fields(self, run, output_path):
112112
f" --sonar-issues-json={self.sonar_issues_json}"
113113
if self.sonar_issues_json
114114
else ""
115+
) + (
116+
f" --sonar-hotspots-json={self.sonar_hotspots_json}"
117+
if self.sonar_hotspots_json
118+
else ""
115119
)
116120
assert run["directory"] == os.path.abspath(self.code_dir)
117121
assert run["sarifs"] == []
@@ -126,11 +130,12 @@ def _assert_results_fields(self, results, output_path):
126130
]
127131

128132
assert ("detectionTool" in result) == bool(self.sonar_issues_json)
133+
assert ("detectionTool" in result) == bool(self.sonar_hotspots_json)
129134

130135
# TODO: if/when we add description for each url
131136
for reference in result["references"][
132137
# Last reference for Sonar has a different description
133-
: (-1 if self.sonar_issues_json else None)
138+
: (-1 if self.sonar_issues_json or self.sonar_hotspots_json else None)
134139
]:
135140
assert reference["url"] == reference["description"]
136141

@@ -199,6 +204,8 @@ def test_file_rewritten(self, codetf_schema):
199204

200205
if self.sonar_issues_json:
201206
command.append(f"--sonar-issues-json={self.sonar_issues_json}")
207+
if self.sonar_hotspots_json:
208+
command.append(f"--sonar-hotspots-json={self.sonar_hotspots_json}")
202209

203210
self.write_original_code()
204211
self.write_original_dependencies()
@@ -245,6 +252,7 @@ class SonarIntegrationTest(BaseIntegrationTest):
245252

246253
code_path = NotImplementedError
247254
sonar_issues_json = "tests/samples/sonar_issues.json"
255+
sonar_hotspots_json = "tests/samples/sonar_hotspots.json"
248256

249257
@classmethod
250258
def setup_class(cls):
@@ -274,16 +282,20 @@ def teardown_class(cls):
274282

275283
@classmethod
276284
def check_sonar_issues(cls):
277-
sonar_results = SonarResultSet.from_json(cls.sonar_issues_json)
285+
from codemodder.codemods.sonar import process_sonar_findings
286+
287+
sonar_results = process_sonar_findings(
288+
[cls.sonar_issues_json, cls.sonar_hotspots_json]
289+
)
278290

279291
assert (
280292
cls.codemod.rule_id in sonar_results
281-
), f"Make sure to add a sonar issue for {cls.codemod.rule_id} in {cls.sonar_issues_json}"
293+
), f"Make sure to add a sonar issue/hotspot for {cls.codemod.rule_id} in {cls.sonar_issues_json} or {cls.sonar_hotspots_json}"
282294
results_for_codemod = sonar_results[cls.codemod.rule_id]
283295
file_path = pathlib.Path(cls.code_filename)
284296
assert (
285297
file_path in results_for_codemod
286-
), f"Make sure to add a sonar issue for file `{cls.code_filename}` under rule `{cls.codemod.rule_id}` in {cls.sonar_issues_json}"
298+
), 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}"
287299

288300
def _assert_sonar_fields(self, result):
289301
assert self.codemod_instance._metadata.tool is not None

‎src/codemodder/sarifs.py‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
11
import json
22
from abc import ABCMeta, abstractmethod
3+
from collections import defaultdict
34
from importlib.metadata import entry_points
45
from pathlib import Path
5-
from typing import Optional
6+
from typing import DefaultDict, Optional
67

78
from typing_extensions import Self
89

@@ -18,8 +19,8 @@ def detect(cls, run_data: dict) -> bool:
1819
pass
1920

2021

21-
def detect_sarif_tools(filenames: list[Path]) -> dict[str, list[str]]:
22-
results: dict[str, list[str]] = {}
22+
def detect_sarif_tools(filenames: list[Path]) -> DefaultDict[str, list[str]]:
23+
results: DefaultDict[str, list[str]] = defaultdict(list)
2324

2425
logger.debug("loading registered SARIF tool detectors")
2526
detectors = {
@@ -33,7 +34,7 @@ def detect_sarif_tools(filenames: list[Path]) -> dict[str, list[str]]:
3334
try:
3435
if det.detect(run):
3536
logger.debug("detected %s sarif: %s", name, fname)
36-
results.setdefault(name, []).append(str(fname))
37+
results[name].append(str(fname))
3738
except (KeyError, AttributeError, ValueError):
3839
continue
3940

‎src/codemodder/scripts/generate_docs.py‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,11 @@ class DocMetadata:
319319
guidance_explained=CORE_METADATA["secure-tempfile"].guidance_explained,
320320
need_sarif="Yes (Sonar)",
321321
),
322+
"secure-random-S2245": DocMetadata(
323+
importance=CORE_METADATA["secure-random"].importance,
324+
guidance_explained=CORE_METADATA["secure-random"].guidance_explained,
325+
need_sarif="Yes (Sonar)",
326+
),
322327
}
323328

324329

‎src/codemodder/sonar_results.py‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -12,23 +12,23 @@
1212

1313
class SonarLocation(Location):
1414
@classmethod
15-
def from_issue(cls, issue) -> Self:
16-
location = issue.get("textRange")
15+
def from_result(cls, result) -> Self:
16+
location = result.get("textRange")
1717
start = LineInfo(location.get("startLine"), location.get("startOffset"), "")
1818
end = LineInfo(location.get("endLine"), location.get("endOffset"), "")
19-
file = Path(issue.get("component").split(":")[-1])
19+
file = Path(result.get("component").split(":")[-1])
2020
return cls(file=file, start=start, end=end)
2121

2222

2323
class SonarResult(Result):
2424

2525
@classmethod
26-
def from_issue(cls, issue) -> Self:
27-
rule_id = issue.get("rule", None)
28-
if not rule_id:
26+
def from_result(cls, result) -> Self:
27+
# Sonar issues have `rule` as key while hotspots call it `ruleKey`
28+
if not (rule_id := result.get("rule", None) or result.get("ruleKey", None)):
2929
raise ValueError("Could not extract rule id from sarif result.")
3030

31-
locations: list[Location] = [SonarLocation.from_issue(issue)]
31+
locations: list[Location] = [SonarLocation.from_result(result)]
3232
return cls(rule_id=rule_id, locations=locations)
3333

3434
def match_location(self, pos, node):
@@ -51,9 +51,9 @@ def from_json(cls, json_file: str | Path) -> Self:
5151
data = json.load(file)
5252

5353
result_set = cls()
54-
for issue in data.get("issues"):
55-
if issue["status"].lower() == "open":
56-
result_set.add_result(SonarResult.from_issue(issue))
54+
for result in data.get("issues") or [] + data.get("hotspots") or []:
55+
if result["status"].lower() in ("open", "to_review"):
56+
result_set.add_result(SonarResult.from_result(result))
5757

5858
return result_set
5959
except Exception:

‎src/core_codemods/__init__.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
from .sonar.sonar_remove_assertion_in_pytest_raises import (
5858
SonarRemoveAssertionInPytestRaises,
5959
)
60+
from .sonar.sonar_secure_random import SonarSecureRandom
6061
from .sonar.sonar_tempfile_mktemp import SonarTempfileMktemp
6162
from .sql_parameterization import SQLQueryParameterization
6263
from .str_concat_in_seq_literal import StrConcatInSeqLiteral
@@ -148,5 +149,6 @@
148149
SonarJwtDecodeVerify,
149150
SonarFixMissingSelfOrCls,
150151
SonarTempfileMktemp,
152+
SonarSecureRandom,
151153
],
152154
)

‎src/core_codemods/secure_random.py‎

Lines changed: 36 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,28 @@
1-
from core_codemods.api import Metadata, Reference, ReviewGuidance, SimpleCodemod
1+
from codemodder.codemods.libcst_transformer import (
2+
LibcstResultTransformer,
3+
LibcstTransformerPipeline,
4+
)
5+
from codemodder.codemods.semgrep import SemgrepRuleDetector
6+
from codemodder.codemods.utils_mixin import NameResolutionMixin
7+
from core_codemods.api import CoreCodemod, Metadata, Reference, ReviewGuidance
28

39

4-
class SecureRandom(SimpleCodemod):
5-
metadata = Metadata(
10+
class SecureRandomTransformer(LibcstResultTransformer, NameResolutionMixin):
11+
change_description = (
12+
"Replace random.{func} with more secure secrets library functions."
13+
)
14+
15+
def on_result_found(self, original_node, updated_node):
16+
self.remove_unused_import(original_node)
17+
self.add_needed_import("secrets")
18+
19+
if self.find_base_name(original_node.func) == "random.choice":
20+
return self.update_call_target(updated_node, "secrets")
21+
return self.update_call_target(updated_node, "secrets.SystemRandom()")
22+
23+
24+
SecureRandom = CoreCodemod(
25+
metadata=Metadata(
626
name="secure-random",
727
review_guidance=ReviewGuidance.MERGE_AFTER_CURSORY_REVIEW,
828
summary="Secure Source of Randomness",
@@ -14,22 +34,16 @@ class SecureRandom(SimpleCodemod):
1434
url="https://docs.python.org/3/library/random.html",
1535
),
1636
],
17-
)
18-
19-
detector_pattern = """
20-
- patterns:
21-
- pattern: random.$F(...)
22-
- pattern-not: random.SystemRandom()
23-
- pattern-inside: |
24-
import random
25-
...
26-
"""
27-
28-
change_description = (
29-
"Replace random.{func} with more secure secrets library functions."
30-
)
31-
32-
def on_result_found(self, original_node, updated_node):
33-
self.remove_unused_import(original_node)
34-
self.add_needed_import("secrets")
35-
return self.update_call_target(updated_node, "secrets.SystemRandom()")
37+
),
38+
detector=SemgrepRuleDetector(
39+
"""
40+
- patterns:
41+
- pattern: random.$F(...)
42+
- pattern-not: random.SystemRandom()
43+
- pattern-inside: |
44+
import random
45+
...
46+
"""
47+
),
48+
transformer=LibcstTransformerPipeline(SecureRandomTransformer),
49+
)

0 commit comments

Comments
 (0)