From dcd14175fa5c98cb7ba60ed1f7df79666467b481 Mon Sep 17 00:00:00 2001 From: Dan D'Avella Date: Wed, 13 Mar 2024 17:00:33 -0400 Subject: [PATCH] Add detection tool metadata to CodeTF results (#366) * Add detectionTool field to CodeTF * Add detection tool metadata for Sonar codemods --- src/codemodder/codemods/base_codemod.py | 27 ++++++++++++++++- src/codemodder/codemods/sonar.py | 21 ++++++++----- .../codemods/test/integration_utils.py | 30 +++++++++++++++++-- src/codemodder/codetf.py | 24 +++++++++++++++ src/codemodder/context.py | 1 + .../sonar/sonar_django_json_response_type.py | 8 ++--- .../sonar/sonar_django_receiver_on_top.py | 8 ++--- .../sonar/sonar_exception_without_raise.py | 8 ++--- .../sonar/sonar_fix_assert_tuple.py | 8 ++--- .../sonar/sonar_flask_json_response_type.py | 8 ++--- .../sonar/sonar_jwt_decode_verify.py | 8 ++--- .../sonar_literal_or_new_object_identity.py | 8 ++--- .../sonar/sonar_numpy_nan_equality.py | 8 ++--- ...sonar_remove_assertion_in_pytest_raises.py | 8 ++--- tests/test_codetf.py | 26 +++++++++++++++- 15 files changed, 144 insertions(+), 57 deletions(-) diff --git a/src/codemodder/codemods/base_codemod.py b/src/codemodder/codemods/base_codemod.py index 5b592012f..57f057f35 100644 --- a/src/codemodder/codemods/base_codemod.py +++ b/src/codemodder/codemods/base_codemod.py @@ -1,3 +1,5 @@ +from __future__ import annotations + import functools import importlib.resources from abc import ABCMeta, abstractmethod @@ -11,7 +13,7 @@ from codemodder.code_directory import file_line_patterns from codemodder.codemods.base_detector import BaseDetector from codemodder.codemods.base_transformer import BaseTransformerPipeline -from codemodder.codetf import Reference +from codemodder.codetf import DetectionTool, Reference, Rule from codemodder.context import CodemodExecutionContext from codemodder.file_context import FileContext from codemodder.logging import logger @@ -31,6 +33,15 @@ class Metadata: review_guidance: ReviewGuidance references: list[Reference] = field(default_factory=list) description: str | None = None + tool: ToolMetadata | None = None + + +@dataclass +class ToolMetadata: + name: str + rule_id: str + rule_name: str + rule_url: str class BaseCodemod(metaclass=ABCMeta): @@ -89,6 +100,20 @@ def id(self) -> str: def summary(self): return self._metadata.summary + @property + def detection_tool(self) -> DetectionTool | None: + if self._metadata.tool is None: + return None + + return DetectionTool( + name=self._metadata.tool.name, + rule=Rule( + id=self._metadata.tool.rule_id, + name=self._metadata.tool.rule_name, + url=self._metadata.tool.rule_url, + ), + ) + @cached_property def docs_module(self) -> Traversable: return importlib.resources.files(self.docs_module_path) diff --git a/src/codemodder/codemods/sonar.py b/src/codemodder/codemods/sonar.py index 4dcc04e44..dbb7acaee 100644 --- a/src/codemodder/codemods/sonar.py +++ b/src/codemodder/codemods/sonar.py @@ -1,6 +1,6 @@ from pathlib import Path -from codemodder.codemods.base_codemod import Metadata, Reference +from codemodder.codemods.base_codemod import Metadata, Reference, ToolMetadata from codemodder.codemods.base_detector import BaseDetector from codemodder.codemods.base_transformer import BaseTransformerPipeline from codemodder.context import CodemodExecutionContext @@ -19,9 +19,10 @@ def from_core_codemod( cls, name: str, other: CoreCodemod, - rules: list[str], + rule_id: str, + rule_name: str, + rule_url: str, transformer: BaseTransformerPipeline | None = None, - new_references: list[Reference] | None = None, ): return SonarCodemod( metadata=Metadata( @@ -29,16 +30,20 @@ def from_core_codemod( summary="Sonar: " + other.summary, review_guidance=other._metadata.review_guidance, references=( - other.references - if not new_references - else other.references + new_references + other.references + [Reference(url=rule_url, description=rule_name)] ), - description=f"This codemod acts upon the following Sonar rules: {str(rules)[1:-1]}.\n\n" + description=f"This codemod acts upon the following Sonar rules: {rule_id}.\n\n" + other.description, + tool=ToolMetadata( + name="Sonar", + rule_id=rule_id, + rule_name=rule_name, + rule_url=rule_url, + ), ), transformer=transformer if transformer else other.transformer, detector=SonarDetector(), - requested_rules=rules, + requested_rules=[rule_id], ) diff --git a/src/codemodder/codemods/test/integration_utils.py b/src/codemodder/codemods/test/integration_utils.py index 8deaefd05..d843fcb45 100644 --- a/src/codemodder/codemods/test/integration_utils.py +++ b/src/codemodder/codemods/test/integration_utils.py @@ -95,6 +95,19 @@ def _assert_run_fields(self, run, output_path): assert run["directory"] == os.path.abspath(SAMPLES_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] @@ -104,10 +117,23 @@ def _assert_results_fields(self, results, output_path): for ref in self.codemod_instance.references ] - # TODO: once we add description for each url. - for reference in result["references"]: + assert ("detectionTool" in result) == bool(self.sonar_issues_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) + ]: 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) + assert len(result["changeset"]) == self.num_changed_files # A codemod may change multiple files. For now we will diff --git a/src/codemodder/codetf.py b/src/codemodder/codetf.py index ce9eb327d..a0b09f1b8 100644 --- a/src/codemodder/codetf.py +++ b/src/codemodder/codetf.py @@ -72,10 +72,34 @@ def validate_description(self): return self +class Rule(BaseModel): + id: str + name: str + url: Optional[str] = None + + +class Finding(BaseModel): + id: str + fixed: bool + reason: Optional[str] = None + + @model_validator(mode="after") + def validate_reason(self): + assert self.fixed or self.reason, "reason is required if fixed is False" + return self + + +class DetectionTool(BaseModel): + name: str + rule: Rule + findings: list[Finding] = [] + + class Result(BaseModel): codemod: str summary: str description: str + detectionTool: Optional[DetectionTool] = None references: Optional[list[Reference]] = None properties: Optional[dict] = None failedFiles: Optional[list[str]] = None diff --git a/src/codemodder/context.py b/src/codemodder/context.py index 4824d29a1..1bbf313e4 100644 --- a/src/codemodder/context.py +++ b/src/codemodder/context.py @@ -158,6 +158,7 @@ def compile_results(self, codemods: list[BaseCodemod]) -> list[CodeTFResult]: codemod=codemod.id, summary=codemod.summary, description=self.add_description(codemod), + detectionTool=codemod.detection_tool, references=codemod.references, properties={}, failedFiles=[str(file) for file in self.get_failures(codemod.id)], diff --git a/src/core_codemods/sonar/sonar_django_json_response_type.py b/src/core_codemods/sonar/sonar_django_json_response_type.py index 6ad05f835..21f3f6b3e 100644 --- a/src/core_codemods/sonar/sonar_django_json_response_type.py +++ b/src/core_codemods/sonar/sonar_django_json_response_type.py @@ -1,12 +1,10 @@ -from codemodder.codemods.base_codemod import Reference from codemodder.codemods.sonar import SonarCodemod from core_codemods.django_json_response_type import DjangoJsonResponseType SonarDjangoJsonResponseType = SonarCodemod.from_core_codemod( name="django-json-response-type-S5131", other=DjangoJsonResponseType, - rules=["pythonsecurity:S5131"], - new_references=[ - Reference(url="https://rules.sonarsource.com/python/type/Bug/RSPEC-5131/"), - ], + rule_id="pythonsecurity:S5131", + rule_name="Endpoints should not be vulnerable to reflected XSS attacks (Django)", + rule_url="https://rules.sonarsource.com/python/type/Bug/RSPEC-5131/", ) diff --git a/src/core_codemods/sonar/sonar_django_receiver_on_top.py b/src/core_codemods/sonar/sonar_django_receiver_on_top.py index 55327b974..dbd11f7a9 100644 --- a/src/core_codemods/sonar/sonar_django_receiver_on_top.py +++ b/src/core_codemods/sonar/sonar_django_receiver_on_top.py @@ -1,12 +1,10 @@ -from codemodder.codemods.base_codemod import Reference from codemodder.codemods.sonar import SonarCodemod from core_codemods.django_receiver_on_top import DjangoReceiverOnTop SonarDjangoReceiverOnTop = SonarCodemod.from_core_codemod( name="django-receiver-on-top-S6552", other=DjangoReceiverOnTop, - rules=["python:S6552"], - new_references=[ - Reference(url="https://rules.sonarsource.com/python/type/Bug/RSPEC-6552/"), - ], + rule_id="python:S6552", + rule_name="Django signal handler functions should have the `@receiver` decorator on top of all other decorators", + rule_url="https://rules.sonarsource.com/python/type/Bug/RSPEC-6552/", ) diff --git a/src/core_codemods/sonar/sonar_exception_without_raise.py b/src/core_codemods/sonar/sonar_exception_without_raise.py index befe7dbba..482b39ed5 100644 --- a/src/core_codemods/sonar/sonar_exception_without_raise.py +++ b/src/core_codemods/sonar/sonar_exception_without_raise.py @@ -1,12 +1,10 @@ -from codemodder.codemods.base_codemod import Reference from codemodder.codemods.sonar import SonarCodemod from core_codemods.exception_without_raise import ExceptionWithoutRaise SonarExceptionWithoutRaise = SonarCodemod.from_core_codemod( name="exception-without-raise-S3984", other=ExceptionWithoutRaise, - rules=["python:S3984"], - new_references=[ - Reference(url="https://rules.sonarsource.com/python/type/Bug/RSPEC-3984/"), - ], + rule_id="python:S3984", + rule_name="Exceptions should not be created without being raised", + rule_url="https://rules.sonarsource.com/python/type/Bug/RSPEC-3984/", ) diff --git a/src/core_codemods/sonar/sonar_fix_assert_tuple.py b/src/core_codemods/sonar/sonar_fix_assert_tuple.py index ca2d4bb23..17d814cbd 100644 --- a/src/core_codemods/sonar/sonar_fix_assert_tuple.py +++ b/src/core_codemods/sonar/sonar_fix_assert_tuple.py @@ -1,12 +1,10 @@ -from codemodder.codemods.base_codemod import Reference from codemodder.codemods.sonar import SonarCodemod from core_codemods.fix_assert_tuple import FixAssertTuple SonarFixAssertTuple = SonarCodemod.from_core_codemod( name="fix-assert-tuple-S5905", other=FixAssertTuple, - rules=["python:S5905"], - new_references=[ - Reference(url="https://rules.sonarsource.com/python/type/Bug/RSPEC-5905/"), - ], + rule_id="python:S5905", + rule_name="Assert should not be called on a tuple literal", + rule_url="https://rules.sonarsource.com/python/type/Bug/RSPEC-5905/", ) diff --git a/src/core_codemods/sonar/sonar_flask_json_response_type.py b/src/core_codemods/sonar/sonar_flask_json_response_type.py index 294bfb93f..eb01df07f 100644 --- a/src/core_codemods/sonar/sonar_flask_json_response_type.py +++ b/src/core_codemods/sonar/sonar_flask_json_response_type.py @@ -1,12 +1,10 @@ -from codemodder.codemods.base_codemod import Reference from codemodder.codemods.sonar import SonarCodemod from core_codemods.flask_json_response_type import FlaskJsonResponseType SonarFlaskJsonResponseType = SonarCodemod.from_core_codemod( name="flask-json-response-type-S5131", other=FlaskJsonResponseType, - rules=["pythonsecurity:S5131"], - new_references=[ - Reference(url="https://rules.sonarsource.com/python/type/Bug/RSPEC-5131/"), - ], + rule_id="pythonsecurity:S5131", + rule_name="Endpoints should not be vulnerable to reflected XSS attacks (Flask)", + rule_url="https://rules.sonarsource.com/python/type/Bug/RSPEC-5131/", ) diff --git a/src/core_codemods/sonar/sonar_jwt_decode_verify.py b/src/core_codemods/sonar/sonar_jwt_decode_verify.py index 243df5eea..3dbafab07 100644 --- a/src/core_codemods/sonar/sonar_jwt_decode_verify.py +++ b/src/core_codemods/sonar/sonar_jwt_decode_verify.py @@ -1,6 +1,5 @@ import libcst as cst -from codemodder.codemods.base_codemod import Reference from codemodder.codemods.libcst_transformer import LibcstTransformerPipeline from codemodder.codemods.sonar import SonarCodemod from codemodder.result import fuzzy_column_match, same_line @@ -33,9 +32,8 @@ def match_location(self, pos, result): SonarJwtDecodeVerify = SonarCodemod.from_core_codemod( name="jwt-decode-verify-S5659", other=JwtDecodeVerify, - rules=["python:S5659"], - new_references=[ - Reference(url="https://rules.sonarsource.com/python/RSPEC-5659/"), - ], + rule_id="python:S5659", + rule_name="JWT should be signed and verified", + rule_url="https://rules.sonarsource.com/python/RSPEC-5659/", transformer=LibcstTransformerPipeline(JwtDecodeVerifySonarTransformer), ) diff --git a/src/core_codemods/sonar/sonar_literal_or_new_object_identity.py b/src/core_codemods/sonar/sonar_literal_or_new_object_identity.py index de76de684..b15d8295b 100644 --- a/src/core_codemods/sonar/sonar_literal_or_new_object_identity.py +++ b/src/core_codemods/sonar/sonar_literal_or_new_object_identity.py @@ -1,12 +1,10 @@ -from codemodder.codemods.base_codemod import Reference from codemodder.codemods.sonar import SonarCodemod from core_codemods.literal_or_new_object_identity import LiteralOrNewObjectIdentity SonarLiteralOrNewObjectIdentity = SonarCodemod.from_core_codemod( name="literal-or-new-object-identity-S5796", other=LiteralOrNewObjectIdentity, - rules=["python:S5796"], - new_references=[ - Reference(url="https://rules.sonarsource.com/python/type/Bug/RSPEC-5796/"), - ], + rule_id="python:S5796", + rule_name="New objects should not be created only to check their identity", + rule_url="https://rules.sonarsource.com/python/type/Bug/RSPEC-5796/", ) diff --git a/src/core_codemods/sonar/sonar_numpy_nan_equality.py b/src/core_codemods/sonar/sonar_numpy_nan_equality.py index 0270082c3..06c81be87 100644 --- a/src/core_codemods/sonar/sonar_numpy_nan_equality.py +++ b/src/core_codemods/sonar/sonar_numpy_nan_equality.py @@ -1,12 +1,10 @@ -from codemodder.codemods.base_codemod import Reference from codemodder.codemods.sonar import SonarCodemod from core_codemods.numpy_nan_equality import NumpyNanEquality SonarNumpyNanEquality = SonarCodemod.from_core_codemod( name="numpy-nan-equality-S6725", other=NumpyNanEquality, - rules=["python:S6725"], - new_references=[ - Reference(url="https://rules.sonarsource.com/python/type/Bug/RSPEC-6725/"), - ], + rule_id="python:S6725", + rule_name="Equality checks should not be made against `numpy.nan`", + rule_url="https://rules.sonarsource.com/python/type/Bug/RSPEC-6725/", ) diff --git a/src/core_codemods/sonar/sonar_remove_assertion_in_pytest_raises.py b/src/core_codemods/sonar/sonar_remove_assertion_in_pytest_raises.py index afaf75b2f..3d7f03655 100644 --- a/src/core_codemods/sonar/sonar_remove_assertion_in_pytest_raises.py +++ b/src/core_codemods/sonar/sonar_remove_assertion_in_pytest_raises.py @@ -1,4 +1,3 @@ -from codemodder.codemods.base_codemod import Reference from codemodder.codemods.sonar import SonarCodemod from core_codemods.remove_assertion_in_pytest_raises import ( RemoveAssertionInPytestRaises, @@ -7,8 +6,7 @@ SonarRemoveAssertionInPytestRaises = SonarCodemod.from_core_codemod( name="remove-assertion-in-pytest-raises-S5915", other=RemoveAssertionInPytestRaises, - rules=["python:S5915"], - new_references=[ - Reference(url="https://rules.sonarsource.com/python/type/Bug/RSPEC-5915/"), - ], + rule_id="python:S5915", + rule_name="Assertions should not be made at the end of blocks expecting an exception", + rule_url="https://rules.sonarsource.com/python/type/Bug/RSPEC-5915/", ) diff --git a/tests/test_codetf.py b/tests/test_codetf.py index 7f2acc5fe..17da7ac27 100644 --- a/tests/test_codetf.py +++ b/tests/test_codetf.py @@ -2,10 +2,19 @@ from pathlib import Path import jsonschema +import pydantic import pytest import requests -from codemodder.codetf import Change, ChangeSet, CodeTF, DiffSide, Result +from codemodder.codetf import ( + Change, + ChangeSet, + CodeTF, + DiffSide, + Finding, + Reference, + Result, +) @pytest.fixture(autouse=True) @@ -111,3 +120,18 @@ def test_write_codetf_with_results(tmpdir, mocker, codetf_schema): CodeTF.model_validate_json(data) jsonschema.validate(json.loads(data), codetf_schema) + + +def test_reference_use_url_for_description(): + ref = Reference(url="https://example.com") + assert ref.description == "https://example.com" + + +def test_unfixed_finding_with_no_reason(): + with pytest.raises(pydantic.ValidationError): + Finding(id="test", fixed=False) + + +def test_unfixed_finding_with_reason(): + finding = Finding(id="test", fixed=False, reason="test") + assert finding.reason == "test"