diff --git a/ghascompliance/checks.py b/ghascompliance/checks.py index 18b2c2a..60e7beb 100644 --- a/ghascompliance/checks.py +++ b/ghascompliance/checks.py @@ -2,10 +2,12 @@ import json from datetime import datetime from typing import * +from urllib.parse import unquote from ghastoolkit import ( GitHub, CodeScanning, + Dependency, Dependencies, DependencyGraph, Dependabot, @@ -22,6 +24,30 @@ LICENSES = [os.path.join(__HERE__, "data", "clearlydefined.json")] +def _parse_alert_purl(purl: str) -> Tuple[str, str, str]: + """Parse a versionless (manager, namespace, name) tuple from a Dependabot alert PURL. + + Unlike ``Dependency.fromPurl()``, this does not treat the first ``@`` in the + PURL as a version delimiter, so scoped npm packages such as + ``pkg:npm/@scope/name`` are parsed correctly even when a version is + present. The version, if any, is always the last ``@``-delimited segment + and never contains a ``/``, so only that trailing segment is stripped. + Percent-encoding is also normalized. + """ + pkg = unquote(purl) + if pkg.startswith("pkg:"): + pkg = pkg[len("pkg:") :] + + if "@" in pkg: + candidate, _, version = pkg.rpartition("@") + if candidate and "/" not in version: + pkg = candidate + + manager, _, rest = pkg.partition("/") + namespace, _, name = rest.rpartition("/") + return manager.lower(), namespace.lower(), name.lower() + + class Checks: def __init__( self, @@ -221,11 +247,16 @@ def checkDependabot(self): "Dependabot REST API returned 400; retrying with GraphQL alerts API" ) alerts = dependabot.getAlertsGraphQL() - # Dependencies are only needed to resolve the alerts, skip the # (expensive) Dependency Graph requests if there are no alerts if alerts: - dependencies = depgraph.getDependencies() + try: + dependencies = depgraph.getDependencies() + except GHASToolkitError as err: + Octokit.warning( + f"Dependency Graph API request failed with status {err.status}; processing Dependabot alerts without dependency enrichment" + ) + dependencies = None else: Octokit.debug( "No Dependabot alerts, skipping Dependency Graph requests" @@ -245,13 +276,35 @@ def checkDependabot(self): continue # Find the dependency from the graph - dependency = dependencies.findPurl(alert.purl) - - if not dependency: - Octokit.error( - f"Unable to find alert in DependencyGraph :: {alert.purl}" + dependency = None + if dependencies: + alert_manager, alert_namespace, alert_name = _parse_alert_purl( + alert.purl + ) + alert_purl = ( + f"pkg:{alert_manager}/{alert_namespace}/{alert_name}" + if alert_namespace + else f"pkg:{alert_manager}/{alert_name}" + ) + alert_fullname = ( + f"{alert_namespace}/{alert_name}" if alert_namespace else alert_name + ) + dependency = next( + ( + dep + for dep in dependencies + if unquote(dep.getPurl(version=False)).lower() == alert_purl + or ( + (dep.manager or "").lower() == alert_manager + and unquote(dep.fullname).lower() == alert_fullname + ) + ), + None, + ) + if not dependency and dependencies is not None: + Octokit.warning( + f"Unable to find alert in DependencyGraph :: {alert.purl}. Continuing with alert package URL" ) - continue severity = alert.severity.lower() @@ -268,9 +321,9 @@ def checkDependabot(self): names = [ # org.apache.commons - dependency.fullname, + dependency.fullname if dependency else alert.purl, #  maven://org.apache.commons - dependency.getPurl(version=False), + dependency.getPurl(version=False) if dependency else alert.purl, ] if self.policy.checkViolation( diff --git a/tests/test_checks.py b/tests/test_checks.py index 078dd3d..c682a03 100644 --- a/tests/test_checks.py +++ b/tests/test_checks.py @@ -1,12 +1,16 @@ import os import sys import unittest +from datetime import datetime +from types import SimpleNamespace from unittest.mock import MagicMock, patch sys.path.append(".") -from ghastoolkit import GitHub +from ghastoolkit import Dependencies, GitHub +from ghastoolkit.errors import GHASToolkitError from ghastoolkit.octokit.graphql import QUERIES +from ghastoolkit.supplychain.dependency import Dependency from ghascompliance import checks as checks_module from ghascompliance.checks import Checks @@ -19,12 +23,18 @@ def testNoLocalQueryOverrides(self): path = os.path.join( os.path.dirname(checks_module.__file__), "octokit", "graphql" ) - self.assertFalse(os.path.exists(path)) + query_files = [] + if os.path.exists(path): + query_files = [ + name for name in os.listdir(path) if name.endswith(".graphql") + ] + self.assertEqual(query_files, []) def testDependencyInfoQueryIsPaginated(self): """Un-paginated queries time out (502) on large repositories.""" query = QUERIES.get("GetDependencyInfo", "") + self.assertTrue(query) self.assertIn("dependencyGraphManifests(first:", query) self.assertIn("$manifests_cursor", query) self.assertIn("$dependencies_cursor", query) @@ -32,13 +42,25 @@ def testDependencyInfoQueryIsPaginated(self): class TestDependabotChecks(unittest.TestCase): def setUp(self) -> None: + super().setUp() GitHub.init( "advanced-security/policy-as-code", reference="refs/heads/main", retrieve_metadata=False, ) self.checks = Checks(Policy("error")) - super().setUp() + + def _create_alert(self): + advisory = SimpleNamespace(ghsa_id="GHSA-test-1234", cwes=["CWE-79"]) + alert = MagicMock() + alert.purl = "pkg:pip/flask" + alert.severity = "critical" + alert.advisory = advisory + alert.createdAt.return_value = datetime(2026, 8, 18, 0, 0, 0) + alert.get.side_effect = lambda key, default=None: {"dismissReason": None}.get( + key, default + ) + return alert def testSkipDependencyGraphWithoutAlerts(self): depgraph = MagicMock() @@ -53,12 +75,10 @@ def testSkipDependencyGraphWithoutAlerts(self): depgraph.getDependencies.assert_not_called() def testFetchDependencyGraphWithAlerts(self): - alert = MagicMock() - alert.get.return_value = None - alert.purl = "pkg:npm/lodash" + alert = self._create_alert() depgraph = MagicMock() - depgraph.getDependencies.return_value.findPurl.return_value = None + depgraph.getDependencies.return_value = Dependencies() dependabot = MagicMock() dependabot.getAlerts.return_value = [alert] @@ -66,5 +86,170 @@ def testFetchDependencyGraphWithAlerts(self): with patch.object(checks_module, "DependencyGraph", return_value=depgraph): violations = self.checks.checkDependabot() - self.assertEqual(violations, 0) + self.assertEqual(violations, 1) depgraph.getDependencies.assert_called_once() + + @patch("ghascompliance.checks.GitHub.repository") + @patch("ghascompliance.checks.DependencyGraph") + @patch("ghascompliance.checks.Dependabot") + @patch("ghascompliance.checks.Octokit.warning") + def test_check_dependabot_dependency_graph_400_still_processes_alerts( + self, warning_mock, dependabot_cls, depgraph_cls, repository_mock + ): + repository_mock.isInPullRequest.return_value = False + + alert = self._create_alert() + dependabot = dependabot_cls.return_value + dependabot.graphql = MagicMock() + dependabot.getAlerts.return_value = [alert] + + depgraph = depgraph_cls.return_value + depgraph.getDependencies.side_effect = GHASToolkitError( + "Bad Request", status=400 + ) + + policy = MagicMock() + policy.checkViolation.return_value = True + + checks = Checks(policy) + self.assertEqual(checks.checkDependabot(), 1) + policy.checkViolation.assert_called_once() + self.assertFalse( + any( + "Unable to find alert in DependencyGraph" in call.args[0] + for call in warning_mock.call_args_list + ) + ) + + @patch("ghascompliance.checks.GitHub.repository") + @patch("ghascompliance.checks.DependencyGraph") + @patch("ghascompliance.checks.Dependabot") + @patch("ghascompliance.checks.Octokit.warning") + def test_check_dependabot_alert_without_dependency_graph_match_uses_purl( + self, warning_mock, dependabot_cls, depgraph_cls, repository_mock + ): + repository_mock.isInPullRequest.return_value = False + + alert = self._create_alert() + dependabot = dependabot_cls.return_value + dependabot.graphql = MagicMock() + dependabot.getAlerts.return_value = [alert] + + alert.purl = "pkg:pip/flask@3.0.0" + dependencies = Dependencies([Dependency(name="requests", manager="pip")]) + depgraph = depgraph_cls.return_value + depgraph.getDependencies.return_value = dependencies + + policy = MagicMock() + policy.checkViolation.return_value = True + + checks = Checks(policy) + self.assertEqual(checks.checkDependabot(), 1) + self.assertEqual( + policy.checkViolation.call_args.kwargs["names"], + [alert.purl, alert.purl], + ) + warning_mock.assert_called_once_with( + f"Unable to find alert in DependencyGraph :: {alert.purl}. Continuing with alert package URL" + ) + + @patch("ghascompliance.checks.GitHub.repository") + @patch("ghascompliance.checks.DependencyGraph") + @patch("ghascompliance.checks.Dependabot") + @patch("ghascompliance.checks.Octokit.warning") + def test_check_dependabot_purl_match_is_case_insensitive( + self, warning_mock, dependabot_cls, depgraph_cls, repository_mock + ): + repository_mock.isInPullRequest.return_value = False + + alert = self._create_alert() + alert.purl = "pkg:nuget/newtonsoft.json@13.0.3" + dependabot = dependabot_cls.return_value + dependabot.graphql = MagicMock() + dependabot.getAlerts.return_value = [alert] + + dependency = Dependency(name="Newtonsoft.Json", manager="nuget") + dependencies = Dependencies([dependency]) + depgraph = depgraph_cls.return_value + depgraph.getDependencies.return_value = dependencies + + policy = MagicMock() + policy.checkViolation.return_value = True + + checks = Checks(policy) + self.assertEqual(checks.checkDependabot(), 1) + self.assertEqual( + policy.checkViolation.call_args.kwargs["names"], + [dependency.fullname, dependency.getPurl(version=False)], + ) + warning_mock.assert_not_called() + + @patch("ghascompliance.checks.GitHub.repository") + @patch("ghascompliance.checks.DependencyGraph") + @patch("ghascompliance.checks.Dependabot") + @patch("ghascompliance.checks.Octokit.warning") + def test_check_dependabot_maven_alert_matches_dependency_fullname( + self, warning_mock, dependabot_cls, depgraph_cls, repository_mock + ): + repository_mock.isInPullRequest.return_value = False + + alert = self._create_alert() + alert.purl = "pkg:maven/org.apache.commons:commons-lang3@3.14.0" + dependabot = dependabot_cls.return_value + dependabot.graphql = MagicMock() + dependabot.getAlerts.return_value = [alert] + + dependency = Dependency( + name="commons-lang3", + namespace="org.apache.commons", + manager="maven", + ) + dependencies = Dependencies([dependency]) + depgraph = depgraph_cls.return_value + depgraph.getDependencies.return_value = dependencies + + policy = MagicMock() + policy.checkViolation.return_value = True + + checks = Checks(policy) + self.assertEqual(checks.checkDependabot(), 1) + self.assertEqual( + policy.checkViolation.call_args.kwargs["names"], + [dependency.fullname, dependency.getPurl(version=False)], + ) + warning_mock.assert_not_called() + + @patch("ghascompliance.checks.GitHub.repository") + @patch("ghascompliance.checks.DependencyGraph") + @patch("ghascompliance.checks.Dependabot") + @patch("ghascompliance.checks.Octokit.warning") + def test_check_dependabot_scoped_npm_alert_matches_dependency_fullname( + self, warning_mock, dependabot_cls, depgraph_cls, repository_mock + ): + repository_mock.isInPullRequest.return_value = False + + alert = self._create_alert() + alert.purl = "pkg:npm/@scope/name@1.2.3" + dependabot = dependabot_cls.return_value + dependabot.graphql = MagicMock() + dependabot.getAlerts.return_value = [alert] + + dependency = Dependency(name="name", namespace="@scope", manager="npm") + dependencies = Dependencies([dependency]) + depgraph = depgraph_cls.return_value + depgraph.getDependencies.return_value = dependencies + + policy = MagicMock() + policy.checkViolation.return_value = True + + checks = Checks(policy) + self.assertEqual(checks.checkDependabot(), 1) + self.assertEqual( + policy.checkViolation.call_args.kwargs["names"], + [dependency.fullname, dependency.getPurl(version=False)], + ) + warning_mock.assert_not_called() + + +if __name__ == "__main__": + unittest.main()