Fix default path include/exclude behavior; update codemod hierarchy (#780)

* Refactor codemod hierarchy

* Move path include/exclude/default behavior into codemods

* Remove `files_to_analyze` from detector API

* Remove `files_to_analyze` from codemod API

* Fix pygoat integration test

* Better docstrings and type annotations

Author: drdavella

.github/workflows/codemod_pygoat.yml

@@ -38,6 +38,6 @@ jobs:
           repository: pixee/pygoat
           path: pygoat
       - name: Run Codemodder
-        run: codemodder --output output.codetf pygoat
+        run: codemodder --dry-run --output output.codetf pygoat
       - name: Check PyGoat Findings
         run: make pygoat-test

ci_tests/test_pygoat_findings.py

@@ -6,7 +6,9 @@
     "pixee:python/add-requests-timeouts",
     "pixee:python/secure-random",
     "pixee:python/sandbox-process-creation",
+    "pixee:python/subprocess-shell-false",
     "pixee:python/django-session-cookie-secure-off",
+    "pixee:python/django-model-without-dunder-str",
     "pixee:python/harden-pyyaml",
     "pixee:python/django-debug-flag-on",
     "pixee:python/url-sandbox",

src/codemodder/cli.py

@@ -3,7 +3,6 @@
 import sys
 
 from codemodder import __version__
-from codemodder.code_directory import DEFAULT_EXCLUDED_PATHS
 from codemodder.logging import OutputFormat, logger
 from codemodder.registry import CodemodRegistry
 
@@ -143,7 +142,7 @@ def parse_args(argv, codemod_registry: CodemodRegistry):
     parser.add_argument(
         "--path-exclude",
         action=CsvListAction,
-        default=DEFAULT_EXCLUDED_PATHS,
+        default=[],
         help="Comma-separated set of UNIX glob patterns to exclude",
     )
     parser.add_argument(

src/codemodder/code_directory.py

@@ -39,54 +39,63 @@ def file_line_patterns(file_path: str | Path, patterns: Sequence[str]):
     ]
 
 
-def filter_files(names: Sequence[str], patterns: Sequence[str], exclude: bool = False):
+def filter_files(names: list[Path], patterns: Sequence[str], exclude: bool = False):
     patterns = (
         [x.split(":")[0] for x in (patterns or [])]
         if not exclude
         # An excluded line should not cause the entire file to be excluded
         else [x for x in (patterns or []) if ":" not in x]
     )
-    return itertools.chain(*[fnmatch.filter(names, pattern) for pattern in patterns])
+    return itertools.chain(
+        *[fnmatch.filter((str(x) for x in names), pattern) for pattern in patterns]
+    )
+
+
+def files_for_directory(parent_path: Path) -> list[Path]:
+    """
+    Return list of all (non-symlink) file paths within a directory, recursively.
+    """
+    return [
+        path
+        for path in Path(parent_path).rglob("*")
+        if Path(path).is_file() and not Path(path).is_symlink()
+    ]
 
 
 def match_files(
-    parent_path: str | Path,
+    parent_path: Path,
+    input_paths: list[Path],
     exclude_paths: Optional[Sequence[str]] = None,
     include_paths: Optional[Sequence[str]] = None,
-):
+) -> list[Path]:
     """
     Find pattern-matching files starting at the parent_path, recursively.
 
     If a file matches any exclude pattern, it is not matched. If any include
-    patterns are passed in, a file must match `*.py` and at least one include patterns.
+    patterns are passed in, a file must match at least one include patterns.
 
     :param parent_path: str name for starting directory
-    :param exclude_paths: list of UNIX glob patterns to exclude
-    :param include_paths: list of UNIX glob patterns to exclude
+    :param exclude_paths: list of UNIX glob patterns to exclude, uses DEFAULT_EXCLUDED_PATHS if None
+    :param include_paths: list of UNIX glob patterns to exclude, uses DEFAULT_INCLUDED_PATHS if None
 
     :return: list of <pathlib.PosixPath> files found within (including recursively) the parent directory
     that match the criteria of both exclude and include patterns.
     """
-    all_files = [
-        str(Path(path).relative_to(parent_path))
-        for path in Path(parent_path).rglob("*")
-    ]
+    paths = [p.relative_to(parent_path) for p in input_paths]
     included_files = set(
         filter_files(
-            all_files,
+            paths,
             include_paths if include_paths is not None else DEFAULT_INCLUDED_PATHS,
         )
     )
     excluded_files = set(
         filter_files(
-            all_files,
+            paths,
             exclude_paths if exclude_paths is not None else DEFAULT_EXCLUDED_PATHS,
             exclude=True,
         )
     )
 
     return [
-        path
-        for p in sorted(list(included_files - excluded_files))
-        if (path := Path(parent_path).joinpath(p)).is_file()
+        parent_path.joinpath(p) for p in sorted(list(included_files - excluded_files))
     ]

src/codemodder/codemodder.py

@@ -8,7 +8,6 @@
 
 from codemodder import __version__, providers, registry
 from codemodder.cli import parse_args
-from codemodder.code_directory import match_files
 from codemodder.codemods.api import BaseCodemod
 from codemodder.codemods.semgrep import SemgrepRuleDetector
 from codemodder.codetf import CodeTF
@@ -72,38 +71,22 @@ def log_report(context, argv, elapsed_ms, files_to_analyze):
 def apply_codemods(
     context: CodemodExecutionContext,
     codemods_to_run: Sequence[BaseCodemod],
-    semgrep_results: ResultSet,
-    files_to_analyze: list[Path],
 ):
     log_section("scanning")
 
-    if not files_to_analyze:
+    if not context.files_to_analyze:
         logger.info("no files to scan")
         return
 
     if not codemods_to_run:
         logger.info("no codemods to run")
         return
 
-    semgrep_finding_ids = semgrep_results.all_rule_ids()
-
     # run codemods one at a time making sure to respect the given sequence
     for codemod in codemods_to_run:
         # NOTE: this may be used as a progress indicator by upstream tools
         logger.info("running codemod %s", codemod.id)
-
-        if isinstance(codemod.detector, SemgrepRuleDetector):
-            if codemod._internal_name not in semgrep_finding_ids:
-                logger.debug(
-                    "no results from semgrep for %s, skipping analysis",
-                    codemod.id,
-                )
-                continue
-
-            files_to_analyze = semgrep_results.files_for_rule(codemod._internal_name)
-
-        # Non-semgrep codemods ignore the semgrep results
-        codemod.apply(context, files_to_analyze)
+        codemod.apply(context)
         record_dependency_update(context.process_dependencies(codemod.id))
         context.log_changes(codemod.id)
 
@@ -197,37 +180,24 @@ def run(original_args) -> int:
         sast_only=argv.sonar_issues_json or argv.sarif,
     )
 
-    included_paths = argv.path_include or codemod_registry.default_include_paths
-
     log_section("setup")
     log_list(logging.INFO, "running", codemods_to_run, predicate=lambda c: c.id)
-    log_list(logging.INFO, "including paths", included_paths)
+    log_list(logging.INFO, "including paths", context.included_paths)
     log_list(logging.INFO, "excluding paths", argv.path_exclude)
 
-    files_to_analyze: list[Path] = [
-        path
-        for path in match_files(
-            context.directory,
-            argv.path_exclude,
-            included_paths,
-        )
-        if path.is_file() and not path.is_symlink()
-    ]
-
-    full_names = [str(path) for path in files_to_analyze]
-    log_list(logging.DEBUG, "matched files", full_names)
+    log_list(
+        logging.DEBUG, "matched files", (str(path) for path in context.files_to_analyze)
+    )
 
-    semgrep_results: ResultSet = find_semgrep_results(
+    context.semgrep_prefilter_results = find_semgrep_results(
         context,
         codemods_to_run,
-        files_to_analyze,
+        context.find_and_fix_paths,
     )
 
     apply_codemods(
         context,
         codemods_to_run,
-        semgrep_results,
-        files_to_analyze,
     )
 
     elapsed = datetime.datetime.now() - start
@@ -243,7 +213,10 @@ def run(original_args) -> int:
         codetf.write_report(argv.output)
 
     log_report(
-        context, argv, elapsed_ms, [] if not codemods_to_run else files_to_analyze
+        context,
+        argv,
+        elapsed_ms,
+        [] if not codemods_to_run else context.files_to_analyze,
     )
     return 0
 

src/codemodder/codemods/api.py

@@ -5,8 +5,10 @@
 
 from codemodder.codemods.base_codemod import (  # noqa: F401
     BaseCodemod,
+    FindAndFixCodemod,
     Metadata,
     Reference,
+    RemediationCodemod,
     ReviewGuidance,
     ToolMetadata,
     ToolRule,