diff --git a/CHANGELOG.md b/CHANGELOG.md index 57deee3..c306f46 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ All notable changes to SkillEvaluator are documented in this file. ### Fixed +- Unpinned-dependency warnings are no longer suppressed by comparison + operators inside PEP 508 environment markers; requirements such as + `pkg; python_version < "3.13"` are now correctly reported. - `--llm-verify` now refuses to send file context from paths outside the skill root, including `..`, absolute paths, and outbound file symlinks. - Gitleaks path allowlist now skips test/example/fixture/mock directories diff --git a/src/skillevaluator/validators/hygiene.py b/src/skillevaluator/validators/hygiene.py index 76cea31..9d34e9f 100644 --- a/src/skillevaluator/validators/hygiene.py +++ b/src/skillevaluator/validators/hygiene.py @@ -173,10 +173,15 @@ def _check_requirements_file(self, req_file: Path) -> ValidationResult: continue pkg_name = match.group(1).lower() + # Marker comparisons do not constrain the package version. Detect + # direct references before the marker so an "@" inside a marker + # value cannot hide an otherwise unpinned requirement. + requirement_part = line.partition(";")[0] + is_direct_reference = "@" in requirement_part if pkg_name in banned_lower: result.add_error(f"{req_file.name}:{line_num} - Banned package: {pkg_name}") - elif not re.search(r"[=<>!]", line): + elif not is_direct_reference and not re.search(r"[=<>!]", requirement_part): result.add_warning(f"{req_file.name}:{line_num} - Unpinned: {line}") if not result.errors and not result.warnings: diff --git a/tests/validators/test_hygiene.py b/tests/validators/test_hygiene.py index 72a3de1..d4e0e40 100644 --- a/tests/validators/test_hygiene.py +++ b/tests/validators/test_hygiene.py @@ -360,6 +360,65 @@ def test_detects_unpinned_dependencies(self, tmp_path: Path): all_messages = result.errors + result.warnings assert any("unpinned" in m.lower() for m in all_messages) + @pytest.mark.parametrize( + "requirement", + [ + 'requests; python_version < "3.13"', + 'requests; python_version != "3.12"', + 'requests;python_version<"3.13"', + 'requests ; python_version < "3.13"', + 'requests; sys_platform == "win32"', + 'requests; implementation_name == "cpython@corp"', + 'requests[security]; python_version >= "3.9"', + ], + ) + def test_environment_marker_comparisons_do_not_hide_unpinned_requirements( + self, tmp_path: Path, requirement: str + ): + """Marker operators must not count as package version constraints.""" + requirements = tmp_path / "requirements.txt" + requirements.write_text(f"{requirement}\n", encoding="utf-8") + + result = HygieneValidator()._check_requirements_file(requirements) + + assert result.errors == [] + assert result.warnings == [f"requirements.txt:1 - Unpinned: {requirement}"] + + @pytest.mark.parametrize( + "requirement", + [ + 'requests>=2; python_version < "3.13"', + "requests>=2.0,<3.0", + "requests==2.31.0", + "requests~=2.31", + "requests!=2.30.0", + "requests @ https://example.invalid/requests.whl", + "requests @ https://example.invalid/a;v=1/requests.whl", + 'requests @ https://example.invalid/requests.whl ; python_version < "3.13"', + ], + ) + def test_marker_handling_preserves_constrained_and_direct_requirements( + self, tmp_path: Path, requirement: str + ): + """Existing version constraints and direct-reference behavior remain accepted.""" + requirements = tmp_path / "requirements.txt" + requirements.write_text(f"{requirement}\n", encoding="utf-8") + + result = HygieneValidator()._check_requirements_file(requirements) + + assert result.errors == [] + assert result.warnings == [] + + def test_banned_requirement_with_marker_remains_an_error(self, tmp_path: Path): + """Banned-package errors keep precedence over marker-aware warnings.""" + requirements = tmp_path / "requirements.txt" + requirements.write_text('pycrypto; python_version < "3.13"\n', encoding="utf-8") + + result = HygieneValidator()._check_requirements_file(requirements) + + assert result.errors == ["requirements.txt:1 - Banned package: pycrypto"] + assert result.warnings == [] + def test_detects_banned_packages(self, tmp_path: Path): """Test detection of banned/deprecated packages.""" skill_dir = tmp_path / "banned-deps-skill"