Skip to content

Commit

Permalink
Add contextual comment to dependency change
Browse files Browse the repository at this point in the history
  • Loading branch information
drdavella committed Oct 24, 2023
1 parent eb2232c commit dbe622c
Show file tree
Hide file tree
Showing 16 changed files with 138 additions and 53 deletions.
2 changes: 1 addition & 1 deletion integration_tests/base_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ def _assert_results_fields(self, results, output_path):

assert len(change["changes"]) == self.num_changes
line_change = change["changes"][0]
assert line_change["lineNumber"] == self.expected_line_change
assert line_change["lineNumber"] == str(self.expected_line_change)
assert line_change["description"] == self.change_description
assert line_change["packageActions"] == []
assert line_change["properties"] == {}
Expand Down
2 changes: 1 addition & 1 deletion integration_tests/test_process_sandbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,4 @@ class TestProcessSandbox(BaseIntegrationTest):

requirements_path = "tests/samples/requirements.txt"
original_requirements = "# file used to test dependency management\nrequests==2.31.0\nblack==23.7.*\nmypy~=1.4\npylint>1\n"
expected_new_reqs = "# file used to test dependency management\nrequests==2.31.0\nblack==23.7.*\nmypy~=1.4\npylint>1\nsecurity==1.0.1"
expected_new_reqs = "# file used to test dependency management\nrequests==2.31.0\nblack==23.7.*\nmypy~=1.4\npylint>1\nsecurity~=1.2.0"
2 changes: 1 addition & 1 deletion integration_tests/test_url_sandbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,4 @@ class TestUrlSandbox(BaseIntegrationTest):

requirements_path = "tests/samples/requirements.txt"
original_requirements = "# file used to test dependency management\nrequests==2.31.0\nblack==23.7.*\nmypy~=1.4\npylint>1\n"
expected_new_reqs = "# file used to test dependency management\nrequests==2.31.0\nblack==23.7.*\nmypy~=1.4\npylint>1\nsecurity==1.0.1"
expected_new_reqs = "# file used to test dependency management\nrequests==2.31.0\nblack==23.7.*\nmypy~=1.4\npylint>1\nsecurity~=1.2.0"
3 changes: 2 additions & 1 deletion src/codemodder/codemods/base_codemod.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

from codemodder.change import Change
from codemodder.context import CodemodExecutionContext
from codemodder.dependency import Dependency
from codemodder.file_context import FileContext
from codemodder.semgrep import run as semgrep_run

Expand Down Expand Up @@ -100,7 +101,7 @@ def line_exclude(self):
def line_include(self):
return self.file_context.line_include

def add_dependency(self, dependency: str):
def add_dependency(self, dependency: Dependency):
self.file_context.add_dependency(dependency)


Expand Down
5 changes: 3 additions & 2 deletions src/codemodder/context.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
from textwrap import indent

from codemodder.change import ChangeSet
from codemodder.dependency import Dependency
from codemodder.dependency_manager import DependencyManager
from codemodder.executor import CodemodExecutorWrapper
from codemodder.logging import logger, log_list
Expand All @@ -26,7 +27,7 @@
class CodemodExecutionContext: # pylint: disable=too-many-instance-attributes
_results_by_codemod: dict[str, list[ChangeSet]] = {}
_failures_by_codemod: dict[str, list[Path]] = {}
dependencies: dict[str, set[str]] = {}
dependencies: dict[str, set[Dependency]] = {}
directory: Path
dry_run: bool = False
verbose: bool = False
Expand All @@ -53,7 +54,7 @@ def add_result(self, codemod_name, change_set):
def add_failure(self, codemod_name, file_path):
self._failures_by_codemod.setdefault(codemod_name, []).append(file_path)

def add_dependencies(self, codemod_id: str, dependencies: set[str]):
def add_dependencies(self, codemod_id: str, dependencies: set[Dependency]):
self.dependencies.setdefault(codemod_id, set()).update(dependencies)

def get_results(self, codemod_name):
Expand Down
63 changes: 63 additions & 0 deletions src/codemodder/dependency.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
from dataclasses import dataclass

from packaging.requirements import Requirement


@dataclass
class License:
name: str
url: str


@dataclass
class Dependency:
requirement: Requirement
description: str
_license: License
oss_link: str
package_link: str

@property
def name(self) -> str:
return self.requirement.name

@property
def version(self) -> str:
return self.requirement.specifier.__str__().strip()

def build_description(self) -> str:
return f"""{self.description}
License: [{self._license.name}]({self._license.url}) ✅ \
[Open Source]({self.oss_link}) ✅ \
[More facts]({self.package_link})
"""

def __hash__(self):
return hash(self.requirement)


DefusedXML = Dependency(
Requirement("defusedxml~=0.7.1"),
description="""\
This package is [recommended by the Python community](https://docs.python.org/3/library/xml.html#the-defusedxml-package) \
to protect against XML vulnerabilities.\
""",
_license=License(
"PSF-2.0",
"https://opensource.org/license/python-2-0/",
),
oss_link="https://github.com/tiran/defusedxml",
package_link="https://pypi.org/project/defusedxml/",
)

Security = Dependency(
Requirement("security~=1.2.0"),
description="""This library holds security tools for protecting Python API calls.""",
_license=License(
"MIT",
"https://opensource.org/license/MIT/",
),
oss_link="https://github.com/pixee/python-security",
package_link="https://pypi.org/project/security/",
)
35 changes: 23 additions & 12 deletions src/codemodder/dependency_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,14 @@
import difflib
from packaging.requirements import Requirement

from codemodder.change import ChangeSet
from codemodder.change import Change, ChangeSet
from codemodder.dependency import Dependency


class DependencyManager:
parent_directory: Path
_lines: list[str]
_new_requirements: list[str]
_new_requirements: list[Dependency]

def __init__(self, parent_directory: Path):
"""One-time class initialization."""
Expand All @@ -20,13 +21,16 @@ def __init__(self, parent_directory: Path):
self._lines = []
self._new_requirements = []

def add(self, dependencies: list[str]):
@property
def new_requirements(self) -> list[str]:
return [str(x.requirement) for x in self._new_requirements]

def add(self, dependencies: list[Dependency]):
"""add any number of dependencies to the end of list of dependencies."""
for dep_str in dependencies:
dep = Requirement(dep_str)
for dep in dependencies:
if dep not in self.dependencies:
self.dependencies.update({dep: None})
self._new_requirements.append(str(dep))
self.dependencies.update({dep.requirement: None})
self._new_requirements.append(dep)

def write(self, dry_run: bool = False) -> Optional[ChangeSet]:
"""
Expand All @@ -35,19 +39,26 @@ def write(self, dry_run: bool = False) -> Optional[ChangeSet]:
if not (self.dependency_file and self._new_requirements):
return None

updated = self._lines + self._new_requirements + ["\n"]
updated = self._lines + self.new_requirements + ["\n"]

diff = "".join(difflib.unified_diff(self._lines, updated))
# TODO: add a change entry for each new requirement
# TODO: make sure to set the contextual_description=True in the properties bag

changes = [
Change(
lineNumber=len(self._lines) + i + 1,
description=dep.build_description(),
properties={"contextual_description": True},
)
for i, dep in enumerate(self._new_requirements)
]

if not dry_run:
with open(self.dependency_file, "w", encoding="utf-8") as f:
f.writelines(self._lines)
f.writelines(self._new_requirements)
f.writelines(self.new_requirements)

self.dependency_file_changed = True
return ChangeSet(str(self.dependency_file), diff, changes=[])
return ChangeSet(str(self.dependency_file), diff, changes=changes)

@property
def found_dependency_file(self) -> bool:
Expand Down
5 changes: 3 additions & 2 deletions src/codemodder/file_context.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from typing import Dict, List

from codemodder.change import Change
from codemodder.dependency import Dependency


@dataclass
Expand All @@ -15,8 +16,8 @@ class FileContext:
line_exclude: List[int] = field(default_factory=list)
line_include: List[int] = field(default_factory=list)
results_by_id: Dict = field(default_factory=dict)
dependencies: set[str] = field(default_factory=set)
dependencies: set[Dependency] = field(default_factory=set)
codemod_changes: List[Change] = field(default_factory=list)

def add_dependency(self, dependency: str):
def add_dependency(self, dependency: Dependency):
self.dependencies.add(dependency)
3 changes: 2 additions & 1 deletion src/core_codemods/process_creation_sandbox.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import libcst as cst
from codemodder.codemods.base_codemod import ReviewGuidance
from codemodder.codemods.api import SemgrepCodemod
from codemodder.dependency import Security


class ProcessSandbox(SemgrepCodemod):
Expand Down Expand Up @@ -42,7 +43,7 @@ def rule(cls):

def on_result_found(self, original_node, updated_node):
self.add_needed_import("security", "safe_command")
self.add_dependency("security==1.0.1")
self.add_dependency(Security)
return self.update_call_target(
updated_node,
"safe_command",
Expand Down
8 changes: 4 additions & 4 deletions src/core_codemods/url_sandbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
from codemodder.codemods.transformations.remove_unused_imports import (
RemoveUnusedImportsCodemod,
)
from codemodder.dependency import Security

replacement_package = "security"
replacement_import = "safe_requests"


Expand Down Expand Up @@ -74,7 +74,7 @@ def transform_module_impl(self, tree: cst.Module) -> cst.Module:
find_requests_visitor.changes_in_file
)
new_tree = tree.visit(ReplaceNodes(find_requests_visitor.nodes_to_change))
self.add_dependency("security==1.0.1")
self.add_dependency(Security)
# if it finds any request.get(...), try to remove the imports
if any(
(
Expand All @@ -84,7 +84,7 @@ def transform_module_impl(self, tree: cst.Module) -> cst.Module:
):
new_tree = AddImportsVisitor(
self.context,
[ImportItem(replacement_package, replacement_import, None, 0)],
[ImportItem(Security.name, replacement_import, None, 0)],
).transform_module(new_tree)
new_tree = RemoveUnusedImportsCodemod(self.context).transform_module(
new_tree
Expand Down Expand Up @@ -122,7 +122,7 @@ def leave_Call(self, original_node: cst.Call):
{
maybe_node: cst.ImportFrom(
module=cst.parse_expression(
replacement_package + "." + replacement_import
f"{Security.name}.{replacement_import}"
),
names=maybe_node.names,
)
Expand Down
3 changes: 2 additions & 1 deletion src/core_codemods/use_defused_xml.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from codemodder.codemods.base_codemod import ReviewGuidance
from codemodder.codemods.api import BaseCodemod
from codemodder.codemods.imported_call_modifier import ImportedCallModifier
from codemodder.dependency import DefusedXML


class DefusedXmlModifier(ImportedCallModifier[Mapping[str, str]]):
Expand Down Expand Up @@ -93,6 +94,6 @@ def transform_module_impl(self, tree: cst.Module) -> cst.Module:
result_tree = visitor.transform_module(tree)
self.file_context.codemod_changes.extend(visitor.changes_in_file)
if visitor.changes_in_file:
self.add_dependency("defusedxml") # TODO: which version?
self.add_dependency(DefusedXML)

return result_tree
3 changes: 2 additions & 1 deletion tests/codemods/base_codemod_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import mock

from codemodder.context import CodemodExecutionContext
from codemodder.dependency import Dependency
from codemodder.file_context import FileContext
from codemodder.registry import CodemodRegistry, CodemodCollection
from codemodder.semgrep import run as semgrep_run
Expand Down Expand Up @@ -49,7 +50,7 @@ def run_and_assert_filepath(self, root, file_path, input_code, expected):

assert output_tree.code == dedent(expected)

def assert_dependency(self, dependency: str):
def assert_dependency(self, dependency: Dependency):
assert self.file_context and self.file_context.dependencies == set([dependency])


Expand Down
11 changes: 6 additions & 5 deletions tests/codemods/test_process_creation_sandbox.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import pytest
from codemodder.dependency import Security
from core_codemods.process_creation_sandbox import ProcessSandbox
from tests.codemods.base_codemod_test import BaseSemgrepCodemodTest

Expand All @@ -22,7 +23,7 @@ def test_import_subprocess(self, tmpdir):
var = "hello"
"""
self.run_and_assert(tmpdir, input_code, expected)
self.assert_dependency("security==1.0.1")
self.assert_dependency(Security)

def test_import_alias(self, tmpdir):
input_code = """import subprocess as sub
Expand All @@ -37,7 +38,7 @@ def test_import_alias(self, tmpdir):
var = "hello"
"""
self.run_and_assert(tmpdir, input_code, expected)
self.assert_dependency("security==1.0.1")
self.assert_dependency(Security)

def test_from_subprocess(self, tmpdir):
input_code = """from subprocess import run
Expand All @@ -52,7 +53,7 @@ def test_from_subprocess(self, tmpdir):
var = "hello"
"""
self.run_and_assert(tmpdir, input_code, expected)
self.assert_dependency("security==1.0.1")
self.assert_dependency(Security)

def test_subprocess_nameerror(self, tmpdir):
input_code = """subprocess.run("echo 'hi'", shell=True)
Expand Down Expand Up @@ -97,7 +98,7 @@ def test_subprocess_nameerror(self, tmpdir):
)
def test_other_import_untouched(self, tmpdir, input_code, expected):
self.run_and_assert(tmpdir, input_code, expected)
self.assert_dependency("security==1.0.1")
self.assert_dependency(Security)

def test_multifunctions(self, tmpdir):
# Test that subprocess methods that aren't part of the codemod are not changed.
Expand All @@ -115,7 +116,7 @@ def test_multifunctions(self, tmpdir):
subprocess.check_output(["ls", "-l"])"""

self.run_and_assert(tmpdir, input_code, expected)
self.assert_dependency("security==1.0.1")
self.assert_dependency(Security)

def test_custom_run(self, tmpdir):
input_code = """from app_funcs import run
Expand Down
Loading

0 comments on commit dbe622c

Please sign in to comment.