From d0c1c83ddb885211ad2f9acffc330c2d8ceddb22 Mon Sep 17 00:00:00 2001 From: Mark2Mac Date: Thu, 30 Jul 2026 23:11:07 +0200 Subject: [PATCH 1/3] fix(supply-chain): only exact pins resolve to a version, ranges do not MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the review on #302: the previous guard still admitted non-exact constraints. `<=8.1.0` matches every earlier release and `==1.*` is a wildcard, so both were handed to the vulnerability lookup as a version the dependency may never install. A vulnerability lookup answers "is THIS release affected?", which is only meaningful when the manifest admits exactly one release. That predicate is now explicit and shared instead of being re-derived at each call site: - `_pinned_version` (PEP 440): only `==` with a fully concrete version. Floors, caps, exclusions, compatible releases and wildcard equality yield None. - `_pinned_npm_version` (semver): only a bare `x.y.z`. npm defaults to caret ranges, so `"^1.8.3"` was being stripped into the concrete release `1.8.3`. Applied to all three extractors — requirements.txt, pyproject.toml and package.json — because the objection in the review holds verbatim for the two that were not touched by the original patch. Note for the maintainer: dropping these specifiers moves more dependencies to version=None, which #318 shows is currently reported as CRITICAL carrying the package's worst-ever advisory. The two fixes are complementary; happy to send the severity side as a separate PR. Regressions cover both cases named in the review (`<=` and `==1.*`) plus the npm caret/tilde/wildcard/range forms. Signed-off-by: Mark2Mac --- .../analyzers/static_patterns_supply_chain.py | 33 +++++++- tests/unit/test_patterns_new.py | 77 +++++++++++++++++++ 2 files changed, 106 insertions(+), 4 deletions(-) diff --git a/src/skillspector/nodes/analyzers/static_patterns_supply_chain.py b/src/skillspector/nodes/analyzers/static_patterns_supply_chain.py index 5a55a32a..69ac192a 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_supply_chain.py +++ b/src/skillspector/nodes/analyzers/static_patterns_supply_chain.py @@ -417,6 +417,32 @@ 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 + return 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]] = [] @@ -427,7 +453,7 @@ def _extract_packages_from_requirements(content: str) -> list[tuple[str, str | N 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 + version = _pinned_version(m.group(2), m.group(3)) results.append((name, version, i)) return results @@ -448,8 +474,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 @@ -495,7 +520,7 @@ def _extract_packages_from_pyproject(content: str) -> list[tuple[str, str | None if not m: continue name = m.group(1) - version = m.group(3) if m.group(2) in ("==", "<=") else None + version = _pinned_version(m.group(2), m.group(3)) idx = content.find(spec) line_num = get_line_number(content, idx) if idx >= 0 else 1 results.append((name, version, line_num)) diff --git a/tests/unit/test_patterns_new.py b/tests/unit/test_patterns_new.py index 9173e499..6878a952 100644 --- a/tests/unit/test_patterns_new.py +++ b/tests/unit/test_patterns_new.py @@ -1391,6 +1391,83 @@ 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_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_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}' From 26b61ce05cc6bc2cd2f128e698affda6a7df516c Mon Sep 17 00:00:00 2001 From: Mark2Mac Date: Thu, 30 Jul 2026 23:16:04 +0200 Subject: [PATCH 2/3] fix(supply-chain): SC4 must not claim a vulnerability it did not verify When a manifest admits a range, no version is resolved and OSV is queried by name alone. The advisories that come back are the package's history, not a match against the release that will be installed: the worst of them may predate every version the range admits. Using that as the finding's severity turns 'setuptools>=61' into a CRITICAL 'Known Vulnerable Dependency'. Scanning 65 skill/plugin units, every SC4 finding in the corpus came from this or from a range being read as a pin (#294/#302). Not one manifest pinned a vulnerable release. The lack of pinning is already reported by SC1, so what is left for SC4 to say is 'could not verify', and it must not outrank a real version match: severity capped at LOW, confidence 0.4, and wording that states the limit instead of implying a match. Version-matched findings are unchanged. Closes #318 Signed-off-by: Mark2Mac --- .../analyzers/static_patterns_supply_chain.py | 29 ++++++++--- tests/unit/test_patterns_new.py | 51 +++++++++++++++++++ 2 files changed, 74 insertions(+), 6 deletions(-) diff --git a/src/skillspector/nodes/analyzers/static_patterns_supply_chain.py b/src/skillspector/nodes/analyzers/static_patterns_supply_chain.py index 69ac192a..3bf65942 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_supply_chain.py +++ b/src/skillspector/nodes/analyzers/static_patterns_supply_chain.py @@ -819,20 +819,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 6878a952..d6331503 100644 --- a/tests/unit/test_patterns_new.py +++ b/tests/unit/test_patterns_new.py @@ -1475,3 +1475,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() From 3c26dcb2958463bde7ca38474847331d45cbea4c Mon Sep 17 00:00:00 2001 From: keshprad <32313895+keshprad@users.noreply.github.com> Date: Mon, 3 Aug 2026 20:05:23 -0700 Subject: [PATCH 3/3] fix(supply-chain): parse PEP 440 requirement pins Signed-off-by: keshprad <32313895+keshprad@users.noreply.github.com> --- pyproject.toml | 1 + .../analyzers/static_patterns_supply_chain.py | 33 ++++++++++++---- tests/unit/test_patterns_new.py | 39 ++++++++++++------- uv.lock | 2 + 4 files changed, 54 insertions(+), 21 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 8ed04d93..b33dab13 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -37,6 +37,7 @@ dependencies = [ "pyyaml>=6.0.1", "pydantic>=2.12.0", "openai>=2.25.0", + "packaging>=24.2", "langgraph>=1.0.10", "langgraph-cli[inmem]>=0.4.14", "langchain-anthropic>=1.4.5", diff --git a/src/skillspector/nodes/analyzers/static_patterns_supply_chain.py b/src/skillspector/nodes/analyzers/static_patterns_supply_chain.py index 3bf65942..b4438675 100644 --- a/src/skillspector/nodes/analyzers/static_patterns_supply_chain.py +++ b/src/skillspector/nodes/analyzers/static_patterns_supply_chain.py @@ -32,6 +32,8 @@ import tomllib from urllib.parse import urlparse +from packaging.requirements import InvalidRequirement, Requirement + 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 @@ -443,6 +445,23 @@ def _pinned_npm_version(spec: str) -> str | None: return None +def _parse_python_requirement(spec: str) -> tuple[str, str | None] | None: + """Return a Python requirement's name and exact pinned version, if any.""" + try: + requirement = Requirement(spec) + except InvalidRequirement: + # Preserve the existing analyzer coverage for malformed requirement + # lines, but never manufacture a version from a partial match. + name_match = re.match(r"^([a-zA-Z][a-zA-Z0-9._-]*)", spec) + return (name_match.group(1), None) if name_match else 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 _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]] = [] @@ -450,10 +469,9 @@ 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 = _pinned_version(m.group(2), m.group(3)) + parsed = _parse_python_requirement(line) + if parsed: + name, version = parsed results.append((name, version, i)) return results @@ -516,11 +534,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: + parsed = _parse_python_requirement(spec) + if not parsed: continue - name = m.group(1) - version = _pinned_version(m.group(2), m.group(3)) + name, version = parsed idx = content.find(spec) line_num = get_line_number(content, idx) if idx >= 0 else 1 results.append((name, version, line_num)) diff --git a/tests/unit/test_patterns_new.py b/tests/unit/test_patterns_new.py index d6331503..6f07e61a 100644 --- a/tests/unit/test_patterns_new.py +++ b/tests/unit/test_patterns_new.py @@ -1395,14 +1395,27 @@ 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("==", "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 # 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 + 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 + + @pytest.mark.parametrize( + ("specifier", "expected_version"), + [ + ("pillow==10.0.0rc1", "10.0.0rc1"), + ("pillow==10.0.0.post1", "10.0.0.post1"), + ("pillow==1!10.0", "1!10.0"), + ], + ) + def test_parse_python_requirement_preserves_pep440_exact_pins( + self, specifier: str, expected_version: str + ) -> None: + assert sc_mod._parse_python_requirement(specifier) == ("pillow", expected_version) def test_pinned_npm_version_rejects_ranges(self) -> None: # npm defaults to caret ranges: stripping the operator turns a range into a concrete @@ -1421,13 +1434,13 @@ 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 + "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" diff --git a/uv.lock b/uv.lock index e796f452..971fa06e 100644 --- a/uv.lock +++ b/uv.lock @@ -2688,6 +2688,7 @@ dependencies = [ { name = "langgraph-cli", extra = ["inmem"] }, { name = "langsmith" }, { name = "openai" }, + { name = "packaging" }, { name = "pydantic" }, { name = "pyyaml" }, { name = "rich" }, @@ -2728,6 +2729,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.2" }, { name = "poetry", marker = "extra == 'dev'", specifier = ">=2.3.0" }, { name = "pydantic", specifier = ">=2.12.0" }, { name = "pytest", marker = "extra == 'dev'", specifier = ">=9.0.0" },