Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sonar tmpfile codemod #393

Merged
merged 4 commits into from
Apr 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion integration_tests/test_sonar_fix_assert_tuple.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
12 changes: 12 additions & 0 deletions integration_tests/test_sonar_tempfile_mktemp.py
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions integration_tests/test_tempfile_mktemp.py
Original file line number Diff line number Diff line change
@@ -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):
Expand All @@ -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
4 changes: 4 additions & 0 deletions src/codemodder/codemods/sonar.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
57 changes: 37 additions & 20 deletions src/codemodder/codemods/test/integration_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import jsonschema

from codemodder import __version__, registry
from codemodder.sonar_results import SonarResultSet

from .validations import execute_code

Expand Down Expand Up @@ -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]
Expand All @@ -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

Expand All @@ -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)
Expand Down Expand Up @@ -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):
Expand All @@ -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):
"""
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 @@ -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(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not necessarily for this PR but we should be able to rewrite this whole section in terms of a dict comprehension and avoid a lot of duplication.

importance=CORE_METADATA["secure-tempfile"].importance,
guidance_explained=CORE_METADATA["secure-tempfile"].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 @@ -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
Expand Down Expand Up @@ -146,5 +147,6 @@
SonarDjangoJsonResponseType,
SonarJwtDecodeVerify,
SonarFixMissingSelfOrCls,
SonarTempfileMktemp,
],
)
10 changes: 10 additions & 0 deletions src/core_codemods/sonar/sonar_tempfile_mktemp.py
Original file line number Diff line number Diff line change
@@ -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/",
)
41 changes: 26 additions & 15 deletions src/core_codemods/tempfile_mktemp.py
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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),
)
40 changes: 40 additions & 0 deletions tests/codemods/test_sonar_tempfile_mktemp.py
Original file line number Diff line number Diff line change
@@ -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))
3 changes: 3 additions & 0 deletions tests/samples/tempfile_mktemp.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import tempfile

tempfile.mktemp()
Loading