Repository: DataDog/guarddog
Pull request #380 (2024-06-07 11:32:08)

Feature: honor requirements versions
closes issue https://github.com/DataDog/guarddog/issues/362
closes issue https://github.com/DataDog/guarddog/issues/262
closes issue https://github.com/DataDog/guarddog/issues/86

**Context**
Guarddog verify command operated against all possible matches of packages. 
So for instance a requirement with `guarddog>=1.0.0`
would result in guarddog verifying all possible matches:
```- 1.0.0
- 1.0.1
- 1.0.2
- ... 
```
This behaviour could lead to a very lengthy scan process and most-likely not the most useful given that only the most recent match is installed by package managers
 
**Changes**
This PR modifies that behaviour to select by default the best "match" (like a real package manager would do)
So, having the following requirement `guarddog>=1.0.0`
Would result for selecting the latest matched version, e.g: `1.9.0`

IMPORTANT: This default behaviour can be overiden setting the environment flag `GUARDDOG_VERIFY_EXHAUSTIVE_DEPENDENCIES` to "True"


Patch:
diff --git a/.gitignore b/.gitignore
index 8c69188..6598423 100644
--- a/.gitignore
+++ b/.gitignore
@@ -12,4 +12,5 @@ wheels/
 *.egg-info/
 build/
 .coverage*
-
+.cache
+.semgrep
diff --git a/guarddog/analyzer/metadata/npm/typosquatting.py b/guarddog/analyzer/metadata/npm/typosquatting.py
index 7322b65..a94ff86 100644
--- a/guarddog/analyzer/metadata/npm/typosquatting.py
+++ b/guarddog/analyzer/metadata/npm/typosquatting.py
@@ -4,10 +4,9 @@ from datetime import datetime, timedelta
 from typing import Optional
 
 from guarddog.analyzer.metadata.typosquatting import TyposquatDetector
+from guarddog.utils.config import TOP_PACKAGES_CACHE_LOCATION
 import requests
 
-TOP_PACKAGES_CACHE_LOCATION = os.environ.get("GUARDDOG_TOP_PACKAGES_CACHE_LOCATION")
-
 
 class NPMTyposquatDetector(TyposquatDetector):
     """Detector for typosquatting attacks. Detects if a package name is a typosquat of one of the top 5000 packages.
diff --git a/guarddog/analyzer/metadata/pypi/typosquatting.py b/guarddog/analyzer/metadata/pypi/typosquatting.py
index dc678ca..40b59af 100644
--- a/guarddog/analyzer/metadata/pypi/typosquatting.py
+++ b/guarddog/analyzer/metadata/pypi/typosquatting.py
@@ -8,14 +8,11 @@ import requests
 import packaging.utils
 
 from guarddog.analyzer.metadata.typosquatting import TyposquatDetector
-
+from guarddog.utils.config import TOP_PACKAGES_CACHE_LOCATION
 
 log = logging.getLogger("guarddog")
 
 
-TOP_PACKAGES_CACHE_LOCATION = os.environ.get('GUARDDOG_TOP_PACKAGES_CACHE_LOCATION')
-
-
 class PypiTyposquatDetector(TyposquatDetector):
     """
     Detector for typosquatting attacks. Detects if a package name is a typosquat of one of the top 1000 packages.
diff --git a/guarddog/scanners/npm_project_scanner.py b/guarddog/scanners/npm_project_scanner.py
index 6424f51..a2373b1 100644
--- a/guarddog/scanners/npm_project_scanner.py
+++ b/guarddog/scanners/npm_project_scanner.py
@@ -1,38 +1,15 @@
 import json
 import logging
-
 import requests
 from semantic_version import NpmSpec, Version  # type:ignore
 
+from guarddog.utils.config import VERIFY_EXHAUSTIVE_DEPENDENCIES
 from guarddog.scanners.npm_package_scanner import NPMPackageScanner
 from guarddog.scanners.scanner import ProjectScanner
 
 log = logging.getLogger("guarddog")
 
 
-def find_all_versions(package_name: str, semver_range: str) -> set[str]:
-    url = f"https://registry.npmjs.org/{package_name}"
-    log.debug(f"Retrieving npm package metadata from {url}")
-    response = requests.get(url)
-    if response.status_code != 200:
-        log.debug(f"No version available, status code {response.status_code}")
-        return set()
-
-    data = response.json()
-    versions = list(data["versions"].keys())
-    log.debug(f"Retrieved versions {', '.join(versions)}")
-    result = set()
-    try:
-        npm_spec = NpmSpec(semver_range)
-    except ValueError:  # not a semver range, let's keep it raw
-        result.add(semver_range)
-        return result
-    for v in versions:
-        if Version(v) in npm_spec:
-            result.add(v)
-    return result
-
-
 class NPMRequirementsScanner(ProjectScanner):
     """
     Scans all packages in the package.json file of a project
@@ -45,12 +22,69 @@ class NPMRequirementsScanner(ProjectScanner):
         super().__init__(NPMPackageScanner())
 
     def parse_requirements(self, raw_requirements: str) -> dict:
+        """
+        Parses requirements.txt specification and finds all valid
+        versions of each dependency
+
+        Args:
+            raw_requirements (str): contents of package file
+
+        Returns:
+            dict: mapping of dependencies to valid versions
+
+            ex.
+            {
+                ....
+                <dependency-name>: [0.0.1, 0.0.2, ...],
+                ...
+            }
+        """
         package = json.loads(raw_requirements)
         dependencies = package["dependencies"] if "dependencies" in package else {}
-        dev_dependencies = package["devDependencies"] if "devDependencies" in package else {}
+        dev_dependencies = (
+            package["devDependencies"] if "devDependencies" in package else {}
+        )
+
+        def get_matched_versions(versions: set[str], semver_range: str) -> set[str]:
+            """
+            Retrieves all versions that match a given semver selector
+            """
+            result = []
+
+            # Filters to specified versions
+            try:
+                spec = NpmSpec(semver_range)
+                result = [Version(m) for m in versions if spec.match(Version(m))]
+            except ValueError:
+                # use it raw
+                return set([semver_range])
+
+            # If just the best matched version scan is required we only keep one
+            if not VERIFY_EXHAUSTIVE_DEPENDENCIES and result:
+                result = [sorted(result).pop()]
+
+            return set([str(r) for r in result])
+
+        def find_all_versions(package_name: str) -> set[str]:
+            """
+            This helper function retrieves all versions availables for the package
+            """
+            url = f"https://registry.npmjs.org/{package_name}"
+            log.debug(f"Retrieving npm package metadata from {url}")
+            response = requests.get(url)
+            if response.status_code != 200:
+                log.debug(f"No version available, status code {response.status_code}")
+                return set()
+
+            data = response.json()
+            versions = set(data["versions"].keys())
+            log.debug(f"Retrieved versions {', '.join(versions)}")
+            return versions
 
         merged = {}  # type: dict[str, set[str]]
-        for package, selector in list(dependencies.items()) + list(dev_dependencies.items()):
+        for package, selector in list(dependencies.items()) + list(
+            dev_dependencies.items()
+        ):
             if package not in merged:
                 merged[package] = set()
             merged[package].add(selector)
@@ -59,7 +93,12 @@ class NPMRequirementsScanner(ProjectScanner):
         for package, all_selectors in merged.items():
             versions = set()  # type: set[str]
             for selector in all_selectors:
-                versions = versions.union(find_all_versions(package, selector))
-            if len(versions) > 0:
-                results[package] = versions
+                versions = versions.union(
+                    get_matched_versions(find_all_versions(package), selector)
+                )
+            if len(versions) == 0:
+                log.error(f"Package/Version {package} not on NPM\n")
+                continue
+
+            results[package] = versions
         return results
diff --git a/guarddog/scanners/pypi_project_scanner.py b/guarddog/scanners/pypi_project_scanner.py
index bb9db4e..578c665 100644
--- a/guarddog/scanners/pypi_project_scanner.py
+++ b/guarddog/scanners/pypi_project_scanner.py
@@ -1,12 +1,13 @@
 import logging
 import re
 import sys
-
 import pkg_resources
 import requests
+from packaging.specifiers import Specifier, Version
 
 from guarddog.scanners.pypi_package_scanner import PypiPackageScanner
 from guarddog.scanners.scanner import ProjectScanner
+from guarddog.utils.config import VERIFY_EXHAUSTIVE_DEPENDENCIES
 
 log = logging.getLogger("guarddog")
 
@@ -47,14 +48,13 @@ class PypiRequirementsScanner(ProjectScanner):
 
         return sanitized_lines
 
-    # FIXME: type return value properly to dict[str, set[str]]
-    def parse_requirements(self, raw_requirements: str) -> dict:
+    def parse_requirements(self, raw_requirements: str) -> dict[str, set[str]]:
         """
         Parses requirements.txt specification and finds all valid
         versions of each dependency
 
         Args:
-            raw_requirements (List[str]): contents of requirements.txt file
+            raw_requirements (str): contents of requirements.txt file
 
         Returns:
             dict: mapping of dependencies to valid versions
@@ -67,19 +67,49 @@ class PypiRequirementsScanner(ProjectScanner):
             }
         """
         requirements = raw_requirements.splitlines()
+        sanitized_requirements = self._sanitize_requirements(requirements)
+        dependencies = {}
 
-        def versions(package_name):
+        def get_matched_versions(versions: set[str], semver_range: str) -> set[str]:
+            """
+            Retrieves all versions that match a given semver selector
+            """
+            result = []
+
+            # Filters to specified versions
+            try:
+                spec = Specifier(semver_range)
+                result = [Version(m) for m in spec.filter(versions)]
+            except ValueError:
+                # use it raw
+                return set([semver_range])
+
+            # If just the best matched version scan is required we only keep one
+            if not VERIFY_EXHAUSTIVE_DEPENDENCIES and result:
+                result = [sorted(result).pop()]
+
+            return set([str(r) for r in result])
+
+        def find_all_versions(package_name: str) -> set[str]:
+            """
+            This helper function retrieves all versions availables for the package
+            """
             url = "https://pypi.org/pypi/%s/json" % (package_name,)
             log.debug(f"Retrieving PyPI package metadata information from {url}")
-            data = requests.get(url).json()
-            versions = sorted(data["releases"].keys(), reverse=True)
+            response = requests.get(url)
+            if response.status_code != 200:
+                log.debug(f"No version available, status code {response.status_code}")
+                return set()
+
+            data = response.json()
+            versions = set(sorted(data["releases"].keys()))
+            log.debug(f"Retrieved versions {', '.join(versions)}")
             return versions
 
-        sanitized_requirements = self._sanitize_requirements(requirements)
-
-        dependencies = {}
-
         def safe_parse_requirements(req):
+            """
+            This helper function yields one valid requirement line at a time
+            """
             parsed = pkg_resources.parse_requirements(req)
             while True:
                 try:
@@ -87,60 +117,34 @@ class PypiRequirementsScanner(ProjectScanner):
                 except StopIteration:
                     break
                 except Exception as e:
-                    sys.stderr.write(f"Error when parsing requirements, received error {str(e)}. This entry will be "
-                                     "ignored.\n")
+                    sys.stderr.write(
+                        f"Error when parsing requirements, received error {str(e)}. This entry will be "
+                        "ignored.\n"
+                    )
                     yield None
 
         try:
             for requirement in safe_parse_requirements(sanitized_requirements):
                 if requirement is None:
                     continue
-                valid_versions = None
-                project_exists_on_pypi = True
-                for spec in requirement.specs:
-                    qualifier, version = spec
-
-                    try:
-                        available_versions = versions(requirement.project_name)  # type: list[str]
-                    except Exception:
-                        sys.stderr.write(f"Package {requirement.project_name} not on PyPI\n")
-                        project_exists_on_pypi = False
-                        continue
-
-                    used_versions = None
-
-                    match qualifier:
-                        case ">":
-                            used_versions = {v for v in available_versions if v > version}
-                        case "<":
-                            used_versions = {v for v in available_versions if v < version}
-                        case ">=":
-                            used_versions = {v for v in available_versions if v >= version}
-                        case "<=":
-                            used_versions = {v for v in available_versions if v <= version}
-                        case "==":
-                            matches = [re.search(version, candidate) for candidate in available_versions]
-                            filtered_matches = list(filter(None, matches))
-                            str_matches = [v.string for v in filtered_matches]
-                            used_versions = set(str_matches)
-                        case "~=":
-                            prefix = "".join(version.split(".")[:-1])
-                            for available_version in available_versions:  # sorted decreasing
-                                if available_version >= version and available_version.startswith(prefix):
-                                    used_versions = set(available_version)
-                                    break
-                        case _:
-                            sys.stderr.write(f"Unknown qualifier: {qualifier}")
-                            continue
-
-                    if valid_versions is None:
-                        valid_versions = used_versions
-                    else:
-                        valid_versions = valid_versions & used_versions
-
-                if project_exists_on_pypi:
-                    dependencies[requirement.project_name] = valid_versions
+
+                versions = get_matched_versions(
+                    find_all_versions(requirement.project_name),
+                    (
+                        requirement.url
+                        if requirement.url
+                        else str(requirement.specifier)
+                    ),
+                )
+
+                if len(versions) == 0:
+                    log.error(
+                        f"Package/Version {requirement.project_name} not on PyPI\n"
+                    )
+                    continue
+
+                dependencies[requirement.project_name] = versions
         except Exception as e:
-            sys.stderr.write(f"Received error {str(e)}")
+            log.error(f"Received error {str(e)}")
 
         return dependencies
diff --git a/guarddog/scanners/scanner.py b/guarddog/scanners/scanner.py
index 17253fb..f275c64 100644
--- a/guarddog/scanners/scanner.py
+++ b/guarddog/scanners/scanner.py
@@ -1,16 +1,14 @@
 import concurrent.futures
 import json
 import logging
-import multiprocessing
 import os
 import sys
 import tempfile
 import typing
 from abc import abstractmethod
 from concurrent.futures import ThreadPoolExecutor
-
 import requests
-
+from guarddog.utils.config import PARALLELISM
 from guarddog.utils.archives import safe_extract
 
 log = logging.getLogger("guarddog")
@@ -25,7 +23,9 @@ class Scanner:
         pass
 
     @abstractmethod
-    def scan_local(self, path, rules=None, callback: typing.Callable[[dict], None] = noop):
+    def scan_local(
+        self, path, rules=None, callback: typing.Callable[[dict], None] = noop
+    ):
         pass
 
 
@@ -53,7 +53,12 @@ class ProjectScanner(Scanner):
             exit(1)
         return (user, personal_access_token)
 
-    def scan_requirements(self, requirements: str, rules=None, callback: typing.Callable[[dict], None] = noop) -> dict:
+    def scan_requirements(
+        self,
+        requirements: str,
+        rules=None,
+        callback: typing.Callable[[dict], None] = noop,
+    ) -> dict:
         """
         Reads the requirements.txt file and scans each possible
         dependency and version
@@ -82,19 +87,14 @@ class ProjectScanner(Scanner):
         def scan_single_dependency(dependency, version):
             log.debug(f"Scanning {dependency} version {version}")
             result = self.package_scanner.scan_remote(dependency, version, rules)
-            return {
-                'dependency': dependency,
-                'version': version,
-                'result': result
-            }
+            return {"dependency": dependency, "version": version, "result": result}
 
         dependencies = self.parse_requirements(requirements)
+        num_workers = PARALLELISM
 
-        num_workers = multiprocessing.cpu_count()
-        if os.environ.get("GUARDDOG_PARALLELISM") is not None:
-            num_workers = int(os.environ["GUARDDOG_PARALLELISM"])
-
-        sys.stderr.write(f"Scanning using at most {num_workers} parallel worker threads\n")
+        sys.stderr.write(
+            f"Scanning using at most {num_workers} parallel worker threads\n"
+        )
         sys.stderr.flush()
         with ThreadPoolExecutor(max_workers=num_workers) as pool:
             try:
@@ -103,12 +103,18 @@ class ProjectScanner(Scanner):
                     assert versions is None or len(versions) > 0
                     if versions is None:
                         # this will cause scan_remote to use the latest version
-                        futures.append(pool.submit(scan_single_dependency, dependency, None))
+                        futures.append(
+                            pool.submit(scan_single_dependency, dependency, None)
+                        )
                     else:
-                        futures.extend(map(
-                            lambda version: pool.submit(scan_single_dependency, dependency, version),
-                            versions
-                        ))
+                        futures.extend(
+                            map(
+                                lambda version: pool.submit(
+                                    scan_single_dependency, dependency, version
+                                ),
+                                versions,
+                            )
+                        )
 
                 results = []
                 for future in concurrent.futures.as_completed(futures):
@@ -157,10 +163,14 @@ class ProjectScanner(Scanner):
         if resp.status_code == 200:
             return self.scan_requirements(resp.content.decode())
         else:
-            sys.stdout.write(f"{req_url} does not exist. Check your link or branch name.")
+            sys.stdout.write(
+                f"{req_url} does not exist. Check your link or branch name."
+            )
             sys.exit(255)
 
-    def scan_local(self, path, rules=None, callback: typing.Callable[[dict], None] = noop):
+    def scan_local(
+        self, path, rules=None, callback: typing.Callable[[dict], None] = noop
+    ):
         """
         Scans a local requirements.txt file
 
@@ -193,7 +203,9 @@ class ProjectScanner(Scanner):
             sys.exit(255)
 
     @abstractmethod
-    def parse_requirements(self, param: str) -> dict[str, set[str]]:  # returns { package: version }
+    def parse_requirements(
+        self, raw_requirements: str
+    ) -> dict[str, set[str]]:  # returns { package: version }
         pass
 
 
@@ -209,7 +221,9 @@ class PackageScanner(Scanner):
         super().__init__()
         self.analyzer = analyzer
 
-    def scan_local(self, path, rules=None, callback: typing.Callable[[dict], None] = noop) -> dict:
+    def scan_local(
+        self, path, rules=None, callback: typing.Callable[[dict], None] = noop
+    ) -> dict:
         """
         Scans local package
 
@@ -232,7 +246,7 @@ class PackageScanner(Scanner):
         if not os.path.exists(path):
             raise Exception(f"Path {path} does not exist.")
 
-        if any(path.endswith(ext) for ext in ('.tar.gz', '.tgz', '.zip', '.whl')):
+        if any(path.endswith(ext) for ext in (".tar.gz", ".tgz", ".zip", ".whl")):
             with tempfile.TemporaryDirectory() as tmpdirname:
                 safe_extract(path, tmpdirname)
                 return self.analyzer.analyze_sourcecode(tmpdirname, rules=rules)
@@ -240,33 +254,47 @@ class PackageScanner(Scanner):
         if os.path.isdir(path):
             return self.analyzer.analyze_sourcecode(path, rules=rules)
 
-        raise Exception(f"Path {path} is not a directory nor an archive type supported by GuardDog.")
+        raise Exception(
+            f"Path {path} is not a directory nor an archive type supported by GuardDog."
+        )
 
     @abstractmethod
-    def download_and_get_package_info(self, directory: str, package_name: str, version=None) -> typing.Tuple[dict, str]:
-        raise NotImplementedError('download_and_get_package_info is not implemented')
-
-    def _scan_remote(self, name, base_dir, version=None, rules=None, write_package_info=False):
+    def download_and_get_package_info(
+        self, directory: str, package_name: str, version=None
+    ) -> typing.Tuple[dict, str]:
+        raise NotImplementedError("download_and_get_package_info is not implemented")
+
+    def _scan_remote(
+        self, name, base_dir, version=None, rules=None, write_package_info=False
+    ):
         directory = os.path.join(os.path.dirname(os.path.abspath(__file__)), base_dir)
 
         file_path = None
         package_info = None
         try:
-            package_info, file_path = self.download_and_get_package_info(directory, name, version)
+            package_info, file_path = self.download_and_get_package_info(
+                directory, name, version
+            )
         except Exception as e:
             log.debug("Unable to download package, ignoring: " + str(e))
-            return {'issues': 0, 'errors': {'download-package': str(e)}}
+            return {"issues": 0, "errors": {"download-package": str(e)}}
 
         results = self.analyzer.analyze(file_path, package_info, rules, name, version)
         if write_package_info:
             package_name = name.replace("/", "-")
-            suffix = f"{package_name}-{version}" if version is not None else package_name
-            with open(os.path.join(results["path"], f'package_info-{suffix}.json'), "w") as file:
+            suffix = (
+                f"{package_name}-{version}" if version is not None else package_name
+            )
+            with open(
+                os.path.join(results["path"], f"package_info-{suffix}.json"), "w"
+            ) as file:
                 file.write(json.dumps(package_info))
 
         return results
 
-    def scan_remote(self, name, version=None, rules=None, base_dir=None, write_package_info=False):
+    def scan_remote(
+        self, name, version=None, rules=None, base_dir=None, write_package_info=False
+    ):
         """
         Scans a remote package
 
@@ -291,7 +319,9 @@ class PackageScanner(Scanner):
 
         with tempfile.TemporaryDirectory() as tmpdirname:
             # Directory to download compressed and uncompressed package
-            return self._scan_remote(name, tmpdirname, version, rules, write_package_info)
+            return self._scan_remote(
+                name, tmpdirname, version, rules, write_package_info
+            )
 
     def download_compressed(self, url, archive_path, target_path):
         """Downloads a compressed file and extracts it
diff --git a/guarddog/utils/config.py b/guarddog/utils/config.py
new file mode 100644
index 0000000..e6eb484
--- /dev/null
+++ b/guarddog/utils/config.py
@@ -0,0 +1,30 @@
+import os
+import multiprocessing
+
+"""
+This parameter specifies the amount of threads to use while requiring parallelism
+- Default: Number of CPUs available
+"""
+PARALLELISM: int = int(
+    os.environ.get("GUARDDOG_PARALLELISM", multiprocessing.cpu_count())
+)
+
+"""
+This flag specifies if an analysis of all posible versions is required
+- True: All possible versions are analyzed
+- False [default]: Only best match is analyzed
+"""
+VERIFY_EXHAUSTIVE_DEPENDENCIES: bool = (
+    os.environ.get("GUARDDOG_VERIFY_EXHAUSTIVE_DEPENDENCIES", "false").lower() == "true"
+)
+
+"""
+This parameter specifies the location of the top packages cache
+- Default: guarddog/analyzer/metadata/resources
+"""
+TOP_PACKAGES_CACHE_LOCATION = os.environ.get(
+    "GUARDDOG_TOP_PACKAGES_CACHE_LOCATION",
+    os.path.abspath(
+        os.path.join(os.path.dirname(__file__), "../analyzer/metadata/resources")
+    ),
+)


Test patch:
diff --git a/tests/core/test_pypi_requirements_scanner.py b/tests/core/test_pypi_requirements_scanner.py
index 1cc9601..e28a990 100644
--- a/tests/core/test_pypi_requirements_scanner.py
+++ b/tests/core/test_pypi_requirements_scanner.py
@@ -1,11 +1,12 @@
-
 from guarddog.scanners.pypi_project_scanner import PypiRequirementsScanner
 
 
 # Regression test for https://github.com/DataDog/guarddog/issues/78
 def test_requirements_scanner():
     scanner = PypiRequirementsScanner()
-    result = scanner.parse_requirements("\n".join(["not-a-real-package==1.0.0", "flask==2.2.2"]))
+    result = scanner.parse_requirements(
+        "\n".join(["not-a-real-package==1.0.0", "flask==2.2.2"])
+    )
     assert "not-a-real-package" not in result  # ignoring non existing packages
     assert "flask" in result
 
@@ -13,12 +14,17 @@ def test_requirements_scanner():
 # Regression test for https://github.com/DataDog/guarddog/issues/88
 def test_requirements_scanner_on_git_url_packages():
     scanner = PypiRequirementsScanner()
-    result = scanner.parse_requirements("\n".join([
-        "flask==2.2.2",
-        "https://wxpython.org/Phoenix/snapshot-builds/wxPython_Phoenix-3.0.3.dev1820+49a8884-cp34-none-win_amd64.whl",
-        "guarddog @ git+https://github.com/DataDog/guarddog.git",
-        "git+https://github.com/DataDog/guarddog.git"
-    ]))
+    result = scanner.parse_requirements(
+        "\n".join(
+            [
+                "flask==2.2.2",
+                "https://wxpython.org/Phoenix/snapshot-builds/wxPython_Phoenix-3.0.3.dev1820+49a8884-cp34-none-win_amd64.whl",
+                "guarddog @ git+https://github.com/DataDog/guarddog.git",
+                "git+https://github.com/DataDog/guarddog.git",
+            ]
+        )
+    )
     assert "guarddog" in result
+    assert "git+https://github.com/DataDog/guarddog.git" in result["guarddog"]
     assert "flask" in result
     assert len(result) == 2