diff --git a/pyproject.toml b/pyproject.toml index 3499a960..50df8c22 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -34,6 +34,7 @@ dependencies = [ "typer>=0.23.0,<0.24", "rich>=14.3.0", "httpx>=0.28.0", + "packaging>=24.0", "pyyaml>=6.0.1", "pydantic>=2.12.0", "openai>=2.25.0", diff --git a/src/skillspector/nodes/analyzers/static_patterns_supply_chain.py b/src/skillspector/nodes/analyzers/static_patterns_supply_chain.py index 5bbba8e2..2f96a097 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_supply_chain.py +++ b/src/skillspector/nodes/analyzers/static_patterns_supply_chain.py @@ -32,6 +32,9 @@ import tomllib from urllib.parse import urlparse +from packaging.requirements import InvalidRequirement, Requirement +from packaging.version import InvalidVersion, Version + from skillspector.inspection_ledger import LedgerOutcome, analyzer_status_for_events, ledger_event from skillspector.logging_config import get_logger from skillspector.models import AnalyzerFinding, Finding, Location, Severity @@ -417,6 +420,55 @@ def _is_typosquat(pkg_name: str, popular: set[str], max_distance: int = 2) -> st } +def _pinned_version(operator: str | None, version: str | None) -> str | None: + """Return *version* only when the specifier pins one concrete release. + + A vulnerability lookup answers "is THIS release affected?". That question is only + meaningful when the manifest admits exactly one release. Under PEP 440 that is ``==`` + with a fully concrete version: floors (``>=``, ``>``), caps (``<=``, ``<``), exclusions + (``!=``), compatible releases (``~=``) and wildcard equality (``==1.*``) all admit more + than one, so the installed version is unknown and must not be passed off as a pin. + """ + if operator != "==" or not version or "*" in version: + return None + try: + Version(version) + except InvalidVersion: + return None + return version + + +def _extract_python_requirement(spec: str) -> tuple[str, str | None] | None: + """Extract a package and a concrete PEP 440 pin from a PEP 508 requirement. + + ``packaging`` parses complete specifiers rather than accepting a numeric prefix. + That keeps valid PEP 440 versions such as ``10.0.0rc1``, ``10.0.0.post1``, + and ``1!10.0`` intact for OSV queries. + """ + try: + requirement = Requirement(spec) + except InvalidRequirement: + return None + + specifiers = list(requirement.specifier) + if len(specifiers) != 1: + return requirement.name, None + specifier = specifiers[0] + return requirement.name, _pinned_version(specifier.operator, specifier.version) + + +def _pinned_npm_version(spec: str) -> str | None: + """Return the pinned version of an npm dependency spec, or None for any range. + + npm defaults to caret ranges, so ``"^1.8.3"`` is *not* a pin: stripping the operator + turns a range into a concrete release that the project may never install. + """ + candidate = spec.strip() + if re.fullmatch(r"\d+\.\d+\.\d+(?:[-+][0-9A-Za-z.-]+)?", candidate): + return candidate + return None + + def _extract_packages_from_requirements(content: str) -> list[tuple[str, str | None, int]]: """Extract (package_name, version_or_None, line_number) from requirements.txt format.""" results: list[tuple[str, str | None, int]] = [] @@ -424,10 +476,13 @@ def _extract_packages_from_requirements(content: str) -> list[tuple[str, str | N line = line.strip() if not line or line.startswith("#") or line.startswith("-"): continue - m = re.match(r"^([a-zA-Z][a-zA-Z0-9._-]*)(?:\[.*?\])?\s*(?:([=<>!~]=?)\s*([\d.*]+))?", line) - if m: - name = m.group(1) - version = m.group(3) if m.group(2) else None + # pip treats a whitespace-prefixed ``#`` as an inline comment, while + # PEP 508 parsing does not. Preserve normal requirements.txt behavior + # before handing the complete requirement to ``packaging``. + line = re.split(r"\s+#", line, maxsplit=1)[0] + requirement = _extract_python_requirement(line) + if requirement: + name, version = requirement results.append((name, version, i)) return results @@ -448,8 +503,7 @@ def _extract_packages_from_package_json(content: str) -> list[tuple[str, str | N m = re.match(r'"([^"]+)"\s*:\s*"([^"]*)"', stripped) if m: name = m.group(1) - ver_str = m.group(2).lstrip("^~>=<") - version = ver_str if re.match(r"^\d", ver_str) else None + version = _pinned_npm_version(m.group(2)) results.append((name, version, i)) return results @@ -491,11 +545,10 @@ def _extract_packages_from_pyproject(content: str) -> list[tuple[str, str | None results: list[tuple[str, str | None, int]] = [] for spec in specs: - m = re.match(r"^([a-zA-Z][a-zA-Z0-9._-]*)(?:\[.*?\])?\s*(?:([=<>!~]=?)\s*([\d.*]+))?", spec) - if not m: + requirement = _extract_python_requirement(spec) + if not requirement: continue - name = m.group(1) - version = m.group(3) if m.group(2) in ("==", "<=") else None + name, version = requirement idx = content.find(spec) line_num = get_line_number(content, idx) if idx >= 0 else 1 results.append((name, version, line_num)) @@ -721,20 +774,37 @@ def _sc4_from_osv( worst_severity = v.severity severity = _osv_severity_to_app(worst_severity) confidence = _SEVERITY_CONFIDENCE.get(worst_severity.upper(), 0.75) - version_str = f"=={pkg_version}" if pkg_version else "" vuln_desc = _format_vuln_ids(vulns) + if pkg_version: + message = ( + f"Known Vulnerable Dependency: {pkg_name}=={pkg_version}" + f" — {len(vulns)} advisory(ies): {vuln_desc}" + ) + matched_text = f"{pkg_name}=={pkg_version}" + else: + # No resolvable version: OSV was queried by name only, so these advisories are + # NOT matched against the release that will actually be installed — they are the + # package's history, and the worst of them may predate every version the range + # admits. Reporting that as the finding's severity turns "setuptools>=61" into a + # CRITICAL. The unpinned dependency itself is already reported by SC1, so what is + # left to say here is "could not verify", and it must not outrank a real match. + severity = Severity.LOW + confidence = 0.4 + message = ( + f"Unverifiable Dependency: {pkg_name} has {len(vulns)} known advisory(ies)" + f" ({vuln_desc}), but the manifest does not pin a version, so it is unknown" + " whether the installed release is affected" + ) + matched_text = pkg_name findings.append( AnalyzerFinding( rule_id="SC4", - message=( - f"Known Vulnerable Dependency: {pkg_name}{version_str}" - f" — {len(vulns)} advisory(ies): {vuln_desc}" - ), + message=message, severity=severity, location=Location(file=file_path, start_line=line_num), confidence=confidence, tags=tag, - matched_text=f"{pkg_name}{version_str}" if version_str else pkg_name, + matched_text=matched_text, ) ) return findings, covered diff --git a/tests/unit/test_patterns_new.py b/tests/unit/test_patterns_new.py index 5c0525d3..df236710 100644 --- a/tests/unit/test_patterns_new.py +++ b/tests/unit/test_patterns_new.py @@ -1262,6 +1262,109 @@ def test_extract_packages_requirements(self) -> None: assert "numpy" in names assert "flask" in names + def test_pinned_version_only_accepts_exact_concrete_pins(self) -> None: + # A vulnerability lookup asks "is THIS release affected?", which is only meaningful + # when the manifest admits exactly one release. Everything else must yield None. + assert sc_mod._pinned_version("==", "2.31.0") == "2.31.0" + assert sc_mod._pinned_version("==", "1.*") is None # wildcard equality + assert sc_mod._pinned_version("<=", "8.1.0") is None # cap: admits every earlier + assert sc_mod._pinned_version("<", "8.1.0") is None + assert sc_mod._pinned_version(">=", "10.0.0") is None # floor + assert sc_mod._pinned_version(">", "10.0.0") is None + assert sc_mod._pinned_version("~=", "1.26.0") is None # compatible release + assert sc_mod._pinned_version("!=", "3.0.0") is None # exclusion + assert sc_mod._pinned_version(None, None) is None # bare dependency + + def test_pinned_npm_version_rejects_ranges(self) -> None: + # npm defaults to caret ranges: stripping the operator turns a range into a concrete + # release the project may never install (regression: "^1.8.3" -> "1.8.3"). + assert sc_mod._pinned_npm_version("4.17.21") == "4.17.21" + assert sc_mod._pinned_npm_version("1.2.3-rc.1") == "1.2.3-rc.1" + assert sc_mod._pinned_npm_version("^1.8.3") is None + assert sc_mod._pinned_npm_version("~4.18.0") is None + assert sc_mod._pinned_npm_version(">=1.2.3") is None + assert sc_mod._pinned_npm_version("1.x") is None + assert sc_mod._pinned_npm_version("*") is None + assert sc_mod._pinned_npm_version(">=1.2.3 <2.0.0") is None + assert sc_mod._pinned_npm_version("") is None + + def test_extract_packages_requirements_specifier_is_not_a_pin(self) -> None: + # Regression: any specifier was treated as "==", so the floor "pillow>=10.0.0" was + # scanned as the exact release 10.0.0 and flagged with that release's CVEs. + content = ( + "requests==2.31.0\n" # exact pin -> kept + "pillow>=10.0.0\n" # floor -> None + "click<=8.1.0\n" # cap -> None + "urllib3~=1.26.0\n" # compatible -> None + "jinja2!=3.0.0\n" # exclusion -> None + "boto3==1.*\n" # wildcard -> None + "flask\n" # unpinned -> None + ) + versions = {p[0]: p[1] for p in sc_mod._extract_packages_from_requirements(content)} + assert versions["requests"] == "2.31.0" + assert versions["pillow"] is None + assert versions["click"] is None + assert versions["urllib3"] is None + assert versions["jinja2"] is None + assert versions["boto3"] is None + assert versions["flask"] is None + + def test_extract_packages_requirements_keeps_full_pep440_pins(self) -> None: + content = ( + "pillow==10.0.0rc1\n" + "pillow-post==10.0.0.post1 # supported post-release pin\n" + "pillow-epoch==1!10.0\n" + ) + versions = {p[0]: p[1] for p in sc_mod._extract_packages_from_requirements(content)} + assert versions == { + "pillow": "10.0.0rc1", + "pillow-post": "10.0.0.post1", + "pillow-epoch": "1!10.0", + } + + def test_extract_packages_pyproject_specifier_is_not_a_pin(self) -> None: + content = ( + "[build-system]\n" + 'requires = ["setuptools>=61", "wheel==0.42.0"]\n' + "[project]\n" + 'dependencies = ["httpx<=0.27.0", "rich==13.*"]\n' + ) + versions = {p[0]: p[1] for p in sc_mod._extract_packages_from_pyproject(content)} + assert versions["wheel"] == "0.42.0" + assert versions["setuptools"] is None + assert versions["httpx"] is None + assert versions["rich"] is None + + def test_extract_packages_pyproject_keeps_full_pep440_pins(self) -> None: + content = ( + "[project]\n" + 'dependencies = ["pillow==10.0.0rc1", "pillow-post==10.0.0.post1", ' + '"pillow-epoch==1!10.0"]\n' + ) + versions = {p[0]: p[1] for p in sc_mod._extract_packages_from_pyproject(content)} + assert versions == { + "pillow": "10.0.0rc1", + "pillow-post": "10.0.0.post1", + "pillow-epoch": "1!10.0", + } + + def test_extract_packages_package_json_caret_is_not_a_pin(self) -> None: + content = ( + "{\n" + ' "dependencies": {\n' + ' "shell-quote": "^1.8.3",\n' + ' "lodash": "4.17.21",\n' + ' "semver": "~7.5.0",\n' + ' "glob": "*"\n' + " }\n" + "}" + ) + versions = {p[0]: p[1] for p in sc_mod._extract_packages_from_package_json(content)} + assert versions["lodash"] == "4.17.21" + assert versions["shell-quote"] is None + assert versions["semver"] is None + assert versions["glob"] is None + def test_extract_packages_package_json(self) -> None: content = ( '{\n "dependencies": {\n "express": "^4.18.0",\n "lodash": "4.17.21"\n }\n}' @@ -1269,3 +1372,54 @@ def test_extract_packages_package_json(self) -> None: names = [p[0] for p in sc_mod._extract_packages_from_package_json(content)] assert "express" in names assert "lodash" in names + + +class TestSC4UnresolvedVersion: + """A name-only OSV query answers a different question than a version match.""" + + @staticmethod + def _vuln(severity: str = "CRITICAL"): + from skillspector.nodes.analyzers.osv_client import VulnResult + + return VulnResult( + vuln_id="GHSA-xxxx-yyyy-zzzz", + summary="historical advisory", + severity=severity, + aliases=("CVE-2020-0001",), + ) + + def test_pinned_version_keeps_osv_severity(self) -> None: + from skillspector.models import Severity + + with patch.object(sc_mod, "query_batch", return_value=[[self._vuln("CRITICAL")]]): + findings, covered = sc_mod._sc4_from_osv( + [("lodash", "4.17.20", 3)], "npm", "package.json", ["supply-chain"] + ) + assert len(findings) == 1 + assert findings[0].severity == Severity.CRITICAL + assert "lodash==4.17.20" in findings[0].message + assert covered == {"lodash"} + + def test_unresolved_version_is_capped_and_reworded(self) -> None: + # "setuptools>=61" resolves to no version, so OSV is queried by name and returns the + # package's history. Reporting the worst of those as the finding's severity claims a + # vulnerability that the installed release may not have. + from skillspector.models import Severity + + with patch.object(sc_mod, "query_batch", return_value=[[self._vuln("CRITICAL")]]): + findings, _ = sc_mod._sc4_from_osv( + [("setuptools", None, 2)], "PyPI", "pyproject.toml", ["supply-chain"] + ) + assert len(findings) == 1 + assert findings[0].severity == Severity.LOW + assert findings[0].confidence < 0.5 + assert "does not pin a version" in findings[0].message + assert "==" not in findings[0].matched_text + + def test_no_vulns_emits_nothing(self) -> None: + with patch.object(sc_mod, "query_batch", return_value=[[]]): + findings, covered = sc_mod._sc4_from_osv( + [("safe-pkg", None, 1)], "PyPI", "requirements.txt", ["supply-chain"] + ) + assert findings == [] + assert covered == set() diff --git a/uv.lock b/uv.lock index bdf5743c..a2e027ea 100644 --- a/uv.lock +++ b/uv.lock @@ -2673,6 +2673,7 @@ dependencies = [ { name = "langgraph-cli", extra = ["inmem"] }, { name = "langsmith" }, { name = "openai" }, + { name = "packaging" }, { name = "pydantic" }, { name = "pyyaml" }, { name = "rich" }, @@ -2711,6 +2712,7 @@ requires-dist = [ { name = "mcp", marker = "extra == 'mcp'", specifier = ">=1.2.0" }, { name = "mypy", marker = "extra == 'dev'", specifier = ">=1.19.0" }, { name = "openai", specifier = ">=2.25.0" }, + { name = "packaging", specifier = ">=24.0" }, { name = "poetry", marker = "extra == 'dev'", specifier = ">=2.3.0" }, { name = "pydantic", specifier = ">=2.12.0" }, { name = "pytest", marker = "extra == 'dev'", specifier = ">=9.0.0" },