-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: read exact versions from Python lockfiles for OSV #263
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -486,6 +486,36 @@ def _extract_packages_from_pyproject(content: str) -> list[tuple[str, str | None | |
| return results | ||
|
|
||
|
|
||
|
|
||
| def _extract_packages_from_toml_lock(content: str) -> list[tuple[str, str | None, int]]: | ||
| """Extract exact package versions from TOML lockfiles such as uv.lock and poetry.lock.""" | ||
| try: | ||
| data = tomllib.loads(content) | ||
| except tomllib.TOMLDecodeError: | ||
| return [] | ||
|
|
||
| packages = data.get("package") | ||
| if not isinstance(packages, list): | ||
| return [] | ||
|
|
||
| results: list[tuple[str, str | None, int]] = [] | ||
| for package in packages: | ||
| if not isinstance(package, dict): | ||
| continue | ||
|
|
||
| name = package.get("name") | ||
| version = package.get("version") | ||
| if not isinstance(name, str) or not name.strip(): | ||
| continue | ||
|
|
||
| version_value = version.strip() if isinstance(version, str) and version.strip() else None | ||
| marker = f'name = "{name}"' | ||
| idx = content.find(marker) | ||
| line_num = get_line_number(content, idx) if idx >= 0 else 1 | ||
| results.append((name, version_value, line_num)) | ||
|
|
||
| return results | ||
|
|
||
| def _version_lt(v1: str, v2: str) -> bool: | ||
| """Simple version comparison: True if v1 < v2 (numeric tuple comparison).""" | ||
|
|
||
|
|
@@ -517,7 +547,7 @@ def ctx(start: int) -> str: | |
|
|
||
| is_dep_file = any( | ||
| n in file_path.lower() | ||
| for n in ["requirements", "package.json", "pyproject.toml", "setup.py", "pipfile"] | ||
| for n in ["requirements", "package.json", "pyproject.toml", "setup.py", "pipfile", "uv.lock", "poetry.lock"] | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Adding |
||
| ) | ||
| if is_dep_file: | ||
| for pattern, confidence in SC1_PATTERNS: | ||
|
|
@@ -761,7 +791,7 @@ def _analyze_dependencies( | |
|
|
||
| lower_path = file_path.lower() | ||
| is_python_dep = any( | ||
| n in lower_path for n in ["requirements", "pyproject.toml", "setup.py", "pipfile"] | ||
| n in lower_path for n in ["requirements", "pyproject.toml", "setup.py", "pipfile", "uv.lock", "poetry.lock"] | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This adds a parallel lockfile scan, but the pyproject.toml/requirements pass still extracts unpinned entries as |
||
| ) | ||
| is_npm_dep = "package.json" in lower_path | ||
|
|
||
|
|
@@ -771,6 +801,8 @@ def _analyze_dependencies( | |
| if is_python_dep: | ||
| if "pyproject.toml" in lower_path: | ||
| packages = _extract_packages_from_pyproject(content) | ||
| elif "uv.lock" in lower_path or "poetry.lock" in lower_path: | ||
| packages = _extract_packages_from_toml_lock(content) | ||
| else: | ||
| packages = _extract_packages_from_requirements(content) | ||
| ecosystem = ECOSYSTEM_PYPI | ||
|
|
@@ -963,7 +995,7 @@ def node(state: SkillspectorState) -> AnalyzerNodeResponse: | |
| lower_path = path.lower() | ||
| is_dep_file = any( | ||
| n in lower_path | ||
| for n in ["requirements", "package.json", "pyproject.toml", "setup.py", "pipfile"] | ||
| for n in ["requirements", "package.json", "pyproject.toml", "setup.py", "pipfile", "uv.lock", "poetry.lock"] | ||
| ) | ||
| if not is_dep_file: | ||
| continue | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| # Licensed under the Apache License, Version 2.0 (the "License"); | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file starts with a UTF-8 BOM (EF BB BF) and uses a single truncated comment line instead of the repo's standard SPDX/Apache-2.0 header (see any sibling test file). It also fails |
||
| from skillspector.nodes.analyzers import static_patterns_supply_chain as supply_chain | ||
| def test_uv_lock_versions_are_passed_to_osv(monkeypatch): | ||
| seen = {} | ||
| def fake_query_batch(packages, ecosystem): | ||
| seen["packages"] = packages | ||
| seen["ecosystem"] = ecosystem | ||
| return [[] for _ in packages] | ||
| monkeypatch.setattr(supply_chain, "query_batch", fake_query_batch) | ||
| content = """ | ||
| version = 1 | ||
| [[package]] | ||
| name = "mlx" | ||
| version = "0.31.2" | ||
| [[package]] | ||
| name = "requests" | ||
| version = "2.31.0" | ||
| """ | ||
| supply_chain._analyze_dependencies(content, "uv.lock") | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Coverage is happy-path only. Consider adding cases for: malformed TOML (returns [] rather than raising), a |
||
| assert seen["ecosystem"] == supply_chain.ECOSYSTEM_PYPI | ||
| assert ("mlx", "0.31.2") in seen["packages"] | ||
| assert ("requests", "2.31.0") in seen["packages"] | ||
| def test_poetry_lock_versions_are_passed_to_osv(monkeypatch): | ||
| seen = {} | ||
| def fake_query_batch(packages, ecosystem): | ||
| seen["packages"] = packages | ||
| seen["ecosystem"] = ecosystem | ||
| return [[] for _ in packages] | ||
| monkeypatch.setattr(supply_chain, "query_batch", fake_query_batch) | ||
| content = """ | ||
| [[package]] | ||
| name = "jinja2" | ||
| version = "3.1.6" | ||
| description = "A fast template engine." | ||
| """ | ||
| supply_chain._analyze_dependencies(content, "poetry.lock") | ||
| assert seen["ecosystem"] == supply_chain.ECOSYSTEM_PYPI | ||
| assert ("jinja2", "3.1.6") in seen["packages"] | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
content.find(marker)returns the first occurrence ofname = "<pkg>", which in uv.lock is frequently adependencies = [{ name = "..." }]entry of an earlier package rather than this package's own[[package]]block, so SC4/SC5/SC6 findings get anchored to the wrong line. Verified with a realistic uv.lock:requestswas attributed to another package's dependencies list. Consider locating the enclosing[[package]]block (e.g., regex over\[\[package\]\]\s*\nname = "...") or tracking positions while parsing.